feat: replace DIFFICULTY with PREVRANDAO after Merge/EIP-4399#162
feat: replace DIFFICULTY with PREVRANDAO after Merge/EIP-4399#162sorpaas merged 1 commit intorust-ethereum:masterfrom
Conversation
vimpunk
changed the title
vimpunk
force-pushed
the
feat/eip-4399
branch
4 times, most recently
from
a3712ca to
5068df6
Compare
sorpaas
left a comment
sorpaas
left a comment
There was a problem hiding this comment.
This PR currently reverted #161, which I don't think is a good thing to do.
The reason we moved config out of runtime because it's actually just not really used. There are huge amount of parameters, yet we just need two of them.
I would appreciate one of the following alternative way:
- Just don't set the
has_prevrandaoconfig. Instead, ifHandler::block_randomnessisSome, userandaobehavior. Otherwise, usedifficultybehavior`. I would prefer this. - Create a separate
RuntimeConfigstruct that just has the three parameters neededstack_limit,memory_limit, andhas_randao.
vimpunk
force-pushed
the
feat/eip-4399
branch
2 times, most recently
from
07d297d to
d19423b
Compare
|
@sorpaas I've addressed your comment. Can you take another look please? |
| fn block_difficulty(&self) -> U256 { | ||
| self.vicinity.block_difficulty | ||
| } | ||
| fn block_randomness(&self) -> Option<U256> { |
There was a problem hiding this comment.
| fn block_randomness(&self) -> Option<U256> { | |
| fn block_randomness(&self) -> Option<H256> { |
And you can avoid the conversion.
There was a problem hiding this comment.
I've changed it but this means EVM hosts (including evm-test) will have to take care of the endianness conversion. Isn't that error-prone?
|
LGTM. Please update the test PR. |
|
Tests PR has been updated to convert endianness: https://github.com/rust-blockchain/evm-tests/pull/19/files#diff-bf42ab7deb71f94483835a445200880b13b91fc19cf43ad526262e4ea4d7b7d3R74-R84 |
|
I think we actually need to merge this first, then I'll be able to update the evm git submodule in the tests PR to the latest master here, and then that can be merged too. |
In the We don't have any CIs in the test repo, only the main one. |
Yes, that's how I've been developing.
I resolved the conflict on that PR. Let me know what to do next. |
|
@sorpaas ping |
|
@sorpaas jsontests won't run I believe, I still need to send one more PR for the two repos to build together. Can we merge this as is and I'll send the follow up PR soon that should fix everything? |
|
That PR has been sent btw: #163 |
* update PrecompileHandle ref: rust-ethereum/evm#122 * update fee calculation ref: rust-ethereum/evm#132 * add code_size/code_hash fn in StackState trait ref: rust-ethereum/evm#140 * update evm call stack ref: rust-ethereum/evm#136 * update evm call stack ref: rust-ethereum/evm#155 * add shanghai eips 3651, 3855, 3860 ref: rust-ethereum/evm#152 * update is_precompile ref: rust-ethereum/evm#157 * fix eip-3860 ref: rust-ethereum/evm#160 * update runtime config ref: rust-ethereum/evm#161 * add eip-4399 ref: rust-ethereum/evm#162 * fix eip-2618 ref: rust-ethereum/evm#163 * fix nonce back to U256 ref: rust-ethereum/evm#166 * remove exit_substate in create functions ref: rust-ethereum/evm#168 * record external cost ref: rust-ethereum/evm#170 * add record_external_operation ref: rust-ethereum/evm#171 * add storage_growth ref: rust-ethereum/evm#173 * update evm * switch to shanghai hardfork * update ecrecover ref: polkadot-evm/frontier#964 (#2696)
2 participants
EIP-4399 that has been introduced in the Merge hasn't yet been implemented in sputnik, so this PR does it.
The EIP changed the DIFFICULTY opcode to PREVRANDAO, that returns random data from a consensus node post Merge, rather than the block difficulty, which after the Merge is always 0.
Changes:
has_prevrandaospecifies whether this EIP is enabled.block_randomnesshas been added toMemoryVicinity, so EVM hosts can specify the randomness they wish to use for the current execution of the EVM. Ifhas_prevrandaois enabled then this value is returned by opcode0x44.ConfigfromRuntimewas reverted as this config is actually needed in this implementation.Note: depends on rust-blockchain/evm-tests#19 to be merged first because CI here is failing as evm-tests is out-of-date.