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.

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

Tests for EIP-3855: PUSH0 instruction #1000

Closed
wants to merge 2 commits into from
Closed

Tests for EIP-3855: PUSH0 instruction #1000

wants to merge 2 commits into from

Conversation

gumb0
Copy link
Member

@gumb0 gumb0 commented Dec 3, 2021

@gumb0 gumb0 force-pushed the push0 branch 2 times, most recently from 0d19c77 to cb76344 Compare December 6, 2021 11:36

0000000000000000000000000000000000000100:
balance: 0
code: ':raw 0x5f'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

save gas before, do (PUSH0) in lllc https://github.com/winsvega/solidity (add as a PR here, retesteth docker uses it)
save the gas after
sstore the gas used change into storage.

b94f5374fce5edbc8e2a8697c15331677e6ebf0b:
storage:
0x01: 0x01
# Same expectation, but for London execution fails on 5f opcode, for 3855 it fails with stack overflow
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

new opcode to be added into opcode tests by Ori.
https://github.com/ethereum/tests/tree/develop/src/GeneralStateTestsFiller/stBadOpcode
and here https://github.com/ethereum/tests/tree/develop/src/Templates/BadOpcode

ask Ori to add this opcode once the realisation is certain

balance: 0
# 1025 push0's
code: ":raw 0x\
5f5f5f5f5f5f5f5f5f5f5f5f5f5f5f5f5f5f5f5f5f5f5f5f5f5f5f5f5f5f5f5f5f5f5f5f5f5f5f5f5f5f5f5f5f5f5f5f5f5f5f5f5f5f5f5f5f5f5f5f5f5f5f5f\
Copy link
Collaborator

@winsvega winsvega Feb 24, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what are you trying to break by calling it 1025 times ?

is there a test cases shit?
I would check push0 in staticcall to see that it act as normal pushes.
also make sure it does not mess up stack if there is some stack pushed already and then push0 happens.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what are you trying to break by calling it 1025 times ?

This case comes from https://eips.ethereum.org/EIPS/eip-3855#test-cases
I think it checks that exactly 1 item is pushed on stack, so 1025 of them overflow stack.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also make sure it does not mess up stack if there is some stack pushed already and then push0 happens.

how would you check this?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also make sure it does not mess up stack if there is some stack pushed already and then push0 happens.

how would you check this?

init:
store: { 01 : 10, 00 : 10 }
code: 60015f55

after the exuction the corresponding cell must change to 0

0000000000000000000000000000000000000100:
balance: 0
# push1(01) push0 sstore
code: ':raw 0x60015f55'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would that also obsolite "0x6000"?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, PUSH1(0) is still valid

@gumb0
Copy link
Member Author

gumb0 commented May 25, 2022

The work continues in another PR #1033

@gumb0 gumb0 closed this May 25, 2022
@axic axic deleted the push0 branch May 25, 2022 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants