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

EIP-2315 changes in gascosts and update opcode #693

Merged

Conversation

matkt
Copy link
Contributor

@matkt matkt commented May 28, 2020

Generated with Besu : hyperledger/besu#995

@matkt matkt changed the title EIP-2315 changes in gastcosts EIP-2315 changes in gascosts May 28, 2020
@holgerd77
Copy link
Contributor

Just asking - without too much insight: there were substantially more test files on the original subroutine test PR #685, were the ones missing not touched by the gas cost changes?

@matkt
Copy link
Contributor Author

matkt commented May 28, 2020

Just asking - without too much insight: there were substantially more test files on the original subroutine test PR #685, were the ones missing not touched by the gas cost changes?

these are the ones that test failed transactions because when a transaction fails, all of the gas available for that transaction (gas limit) is sent to the miner. So the tests will not change

@matkt matkt changed the title EIP-2315 changes in gascosts EIP-2315 changes in gascosts and update opcode May 29, 2020
@winsvega
Copy link
Collaborator

winsvega commented Jun 1, 2020

please update all stSubroutine tests. the previous PR merged was not refilled.
the HF defenition is updated in travis on tests::develop

@matkt
Copy link
Contributor Author

matkt commented Jun 1, 2020

please update all stSubroutine tests. the previous PR merged was not refilled.
the HF defenition is updated in travis on tests::develop

I regenerated the tests with the latest opcodes. I may have missed something. What do you mean by update?

@winsvega
Copy link
Collaborator

winsvega commented Jun 1, 2020

please update all stSubroutine tests. the previous PR merged was not refilled.
the HF defenition is updated in travis on tests::develop

I regenerated the tests with the latest opcodes. I may have missed something. What do you mean by update?

save all the test fillers. rebase on tests develop. paste test fillers back and run filltests command.

@matkt
Copy link
Contributor Author

matkt commented Jun 1, 2020

please update all stSubroutine tests. the previous PR merged was not refilled.
the HF defenition is updated in travis on tests::develop

I regenerated the tests with the latest opcodes. I may have missed something. What do you mean by update?

save all the test fillers. rebase on tests develop. paste test fillers back and run filltests command.

Thanks, Done

@winsvega
Copy link
Collaborator

winsvega commented Jun 1, 2020

hm. travis report errors. did you run
-t GeneralStateTests/stSubroutine -- --filltests
-t GeneralStateTests/stSubroutine -- --filltests --fillchain
?

@matkt
Copy link
Contributor Author

matkt commented Jun 1, 2020

hm. travis report errors. did you run
-t GeneralStateTests/stSubroutine -- --filltests
-t GeneralStateTests/stSubroutine -- --filltests --fillchain
?

Yes I'm doing
./retesteth -t GeneralStateTests/stSubroutine -- --filltests --fillchain
And after
./retesteth -t GeneralStateTests/stSubroutine -- --filltests

@matkt
Copy link
Contributor Author

matkt commented Jun 1, 2020

hm. travis report errors. did you run
-t GeneralStateTests/stSubroutine -- --filltests
-t GeneralStateTests/stSubroutine -- --filltests --fillchain
?

It's strange because when I compare the value of HashSrcwith the filler content, it's the same 😕

@winsvega
Copy link
Collaborator

winsvega commented Jun 1, 2020

This HashSrc is what python script reports. To see what retesteth think is the hash run test with --showhash argument. Apparently python can't calculate the hash of your comment.

‘virtual stop’  is parsed as \u2018virtual stop\u2019 by python script.

@matkt
Copy link
Contributor Author

matkt commented Jun 1, 2020

This HashSrc is what python script reports. To see what retesteth think is the hash run test with --showhash argument. Apparently python can't calculate the hash of your comment.

‘virtual stop’  is parsed as \u2018virtual stop\u2019 by python script.

oh yes 🤦 thanks, I just modified the comment

@sr9000
Copy link
Contributor

sr9000 commented Jun 2, 2020

@matkt Hi! I'm a new intern.
I implemented subroutine opcodes support for LLLC. Now you can use this pr winsvega/lllc#2 to compile LLLC and write evm code for fillers using opcodes instead bytecode.

@matkt
Copy link
Contributor Author

matkt commented Jun 3, 2020

@matkt Hi! I'm a new intern.
I implemented subroutine opcodes support for LLLC. Now you can use this pr winsvega/solidity#2 to compile LLLC and write evm code for fillers using opcodes instead bytecode.

I just used your update to use the LLLC code. there are two tests that I have not changed because the compiler adds a STOP at the end automatically and the goal of the test is to test without STOP at the end.

  • beginSubAtEndOfCode
  • subroutineAtEndOfCode

I also changed the code of the twoLevelsSubroutines test because the generated bytecode became different from the original testcase bytecode (optimization) and it was necessary to update the destination of the jumpsub

  • Expected 0x6800000000000000000c5e005c60115e5d5c5d

  • After LLC compilation of (asm 0x00000000000000000c JUMPSUB STOP BEGINSUB 0x11 JUMPSUB RETURNSUB BEGINSUB RETURNSUB)" -> 0x600c5e005c60115e5d5c5d00

  • The value i'm going to use : (asm 0x04 JUMPSUB STOP BEGINSUB 0x09 JUMPSUB RETURNSUB BEGINSUB RETURNSUB) -> 0x60045e005c60095e5d5c5d00

@winsvega
Copy link
Collaborator

winsvega commented Jun 4, 2020

Thats are important comments. Worth adding to the filler around lll code. Thanks

@winsvega winsvega merged commit 16fa567 into ethereum:develop Jun 5, 2020
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.

4 participants