new(tests): EIP-7251: Increase the MAX_EFFECTIVE_BALANCE (EL Consolidations) #642
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
marioevz
force-pushed
the
eip-7251
branch
2 times, most recently
from
3a41f67 to
f7c1325
Compare
spencer-tb
reviewed
Jun 26, 2024
spencer-tb
reviewed
Jun 26, 2024
spencer-tb
reviewed
Jun 26, 2024
There was a problem hiding this comment.
Tests are looking good from my side. Helped my understanding of the EIP. These align really nicely with the 6110, 7002 & 7685 tests.
Some additional coverage ideas:
- Should we add cases where the source and target public keys are in an invalid format? Assuming t8n rejects filling these cases then I assume we'd want to go the "overwriting tx rlp" route. Maybe we could do this for the other request eips.
- I'm not certain what would happen when we have a duplicate consolidation request. My assumption is that both would be valid on the EL side where the CL would reject one of the requests when adding it to the queue. Nonetheless we could add a duplicate consolidation request test.
- Maybe a test where the target and source keys are the same.
- These might exist here already but could we do recursive consolidation requests? My guess is where we use the same target public key.
I don't understand how could they be recursive since the contracts cannot do calls themselves to other contracts, do you mean on a loop? |
|
Added a modification that ensures the test keeps producing blocks until the consolidation/withdrawal requests queues are exhausted, in order to properly validate that the invalid requests are not included in a future block. This is done to all test cases. |
marioevz
changed the title
Yeah you are right, they can't be recursive! |
spencer-tb
approved these changes
Jul 23, 2024
tests/prague/eip7685_general_purpose_el_requests/test_deposits_withdrawals_consolidations.py
Show resolved
Hide resolved
fix(evm_transition_tool): result type fix
Update src/ethereum_test_tools/common/conversions.py Co-authored-by: spencer <[email protected]>
move contract
fix(tests): EIP-7251 new(tests): Add deposit+withdrawal+consolidation tests new(tests): Add different requests in same tx new(tests): Add invalid-order requests tests with consolidations Update tests/prague/eip7251_consolidations/helpers.py Co-authored-by: spencer <[email protected]> Update tests/prague/eip7251_consolidations/spec.py Co-authored-by: spencer <[email protected]> Update tests/prague/eip7251_consolidations/spec.py Co-authored-by: spencer <[email protected]> tests: add consolidation test with same pubkey fix(tests): EIP-7002,EIP-7251: keep producing empty blocks to exhaust queues fix(tests): tox fix(tests): tox fix
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
🗒️ Description
Implements tests for EIP-7251
🔗 Related Issues
#606
✅ Checklist
mkdocs servelocally and verified the auto-generated docs for new tests in the Test Case Reference are correctly formatted.