Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Merge PoA miner/coinbase address calculation in vm_state and clique-ec_recover #768

Closed
jangko opened this issue Jul 26, 2021 · 2 comments
Closed

Merge PoA miner/coinbase address calculation in vm_state and clique-ec_recover #768

jangko opened this issue Jul 26, 2021 · 2 comments
Labels
high priority refactoring

Comments

@jangko
Copy link
Contributor

jangko commented Jul 26, 2021

nimbus-eth1/nimbus/vm/state.nim

Line 89 in a0d0e35

proc calcMinerAddress(sigRaw: openArray[byte], vmState: BaseVMState, output: var EthAddress): bool =

nimbus-eth1/nimbus/vm2/state.nim

Line 77 in a0d0e35

proc calcMinerAddress(sigRaw: openArray[byte], vmState: BaseVMState, output: var EthAddress): bool =

nimbus-eth1/nimbus/p2p/clique/clique_cfg/ec_recover.nim

Line 54 in a0d0e35

proc(header: BlockHeader): Result[EthAddress,CliqueError] =

we have three different places where the PoA miner address is calculated. time to clean it up and use only one calcucator. ping @mjfh.
@mjfh
Copy link
Contributor

mjfh commented Jul 26, 2021

Good point.

Most part of the ec_recover.toValue() handler can be implemented outside. And the calculation is only dependent on the header (more specifically extraData field.)

Will re-implement it as utils/<something>.nim.

mjfh added a commit that referenced this issue Aug 2, 2021
@mjfh
#768 Moved/re-implemented ecRecover() from Clique sources to utils/ec…
1a198b6 
…_recover

why:
  The same functionality was differently implemented in one or the
  other form.

details:
  Caching and non-caching variants available
jangko pushed a commit that referenced this issue Aug 5, 2021
@mjfh @jangko
#768 Moved/re-implemented ecRecover() from Clique sources to utils/ec…
4713bd4 
…_recover

why:
  The same functionality was differently implemented in one or the
  other form.

details:
  Caching and non-caching variants available
@jangko
Copy link
Contributor Author

jangko commented Aug 5, 2021

fixed by #782, closing

@jangko jangko closed this as completed Aug 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
high priority refactoring
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@jangko @mjfh