-
Notifications
You must be signed in to change notification settings - Fork 328
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.
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
EIP-1153: Transient Storage tests #1091
Conversation
tstore tests 8,9,11,12,13
# Conflicts: # GeneralStateTests/stExample/mergeTest.json
I haven't looked at all the tests but one thing that came to mind when hearing about this EIP is that the clients need to implement a new data structure that might be ephemeral but we might still need to test the maximum number of transient elements it can realistically contain within the span of a single transaction, just to make sure that this structure does not break. |
Mentioned in the EIP security considerations: https://eips.ethereum.org/EIPS/eip-1153#security-considerations And it should be tested in the file |
- data: :abi doCall(uint) 0x200 | ||
accessList: [] | ||
gasLimit: | ||
- "400000" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should probably be much more than 400k, like 30m gas, and 0x200
-> 0x30d40
(200k)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed here d1f8ba8
can this EIP be activated by command |
updated, thank you. can now run them off the develop branch of retesteth with the following commands GeneralStateTests:
BlockchainTests:
|
6a5435d
to
fcd3575
Compare
move stEIP1153/bcEIP1153 into EIPTests
:yul { | ||
switch selector() | ||
|
||
case 0x883264e8 { // doCall(uint) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bit strange how the selector is computed from doCall(uint)
rather than doCall(uint256)
which is how solidity would do it, and that :abi doCall(uint)
on line 72 below takes the signature as a literal instead of parsing it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in other words, this is not consistent with solidity. doesn't really matter, but thought i'd point it out. you also have a function selector computed from keccak256('doNStores(n)')
which is what made me think of it (using n
instead of the type in the signature)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
git reset --soft HEAD~60
Can repack this into smaller number of commits. Like test srcs, test filled separate
We don't need history of editing and moving from folder to folder.
And its ready to merge into EIPTests folder, then we can pr to make it better ib another thread. And eventually update to cancun or when it's gonna be accepted
@winsvega Can we close this in favor of https://github.com/ethereum/tests/pull/1194/files which squashes these commits and is updated against latest develop branch |
Finally ready to merge. Thanks for the contribution! |
No description provided.