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.

Sign up for GitHub

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

Jump to bottom

evm t8n tool to use ExecuteBlockEphemerally api #4512

Merged
merged 24 commits into from
Jul 2, 2022
Merged

evm t8n tool to use ExecuteBlockEphemerally api #4512

merged 24 commits into from
Jul 2, 2022

Conversation

sudeepdino008
Copy link
Contributor

@sudeepdino008 sudeepdino008 commented Jun 21, 2022 •
edited
Loading

  • get evm t8n to use ExecuteBlockEphemerally
  • unit tests to ensure that the outputs are same as corresponding geth's evm tool
  • align structs used by the evm tool to match to the current status of geth's evm tool (for the purpose of updating to
    capture new information and to achieve parity)

@sudeepdino008
fix to set V, R, S in legacy transaction
768423f
@sudeepdino008
fix to dump post-execution alloc for evm t8n
a326df8
@sudeepdino008
close tx in evm t8n
75d3819
@sudeepdino008
populate current difficulty and gas used in output result
8ecb49e 
- update the ExecutionResult to include corresponding info (like
  Difficulty/GasUsed)
@sudeepdino008
initial attempt at migrating 'evm t8n' to use ExecuteBlockEphemerally
6beaf80
@sudeepdino008
using ExecutionResult in ExecuteBlockEphemerally
6011292
@sudeepdino008
bypass validations and integrate with EphemeralExecResult
916ac3e
@sudeepdino008
fixing output of 'evm t8n'
759ee99 
- remaining bits are "stateRoot" in results.txt and "balance" field for one account in
  alloc.txt (for testdata=1)
@sudeepdino008
get ExecuteBlockEphemerally to accept getTracer lambda
b8ea5d1
@sudeepdino008
fix build failure
26c51e2
@sudeepdino008
test cases for evm t8n
77d3df5
@sudeepdino008
more test cases for evm t8n
829adae
@sudeepdino008
fix stateRoot computation in evm t8n
13b4425
@sudeepdino008
remove reward argument, as EBE itself takes care of it
ba1df9f
@sudeepdino008 sudeepdino008 marked this pull request as draft June 21, 2022 14:19
@sudeepdino008 sudeepdino008 marked this pull request as ready for review June 22, 2022 06:29
@sudeepdino008 sudeepdino008 marked this pull request as draft June 22, 2022 16:46
@AlexeyAkhunov
Copy link
Contributor

Thank you! Can you please remove function rlpHash in the file cmd/evm/internal/t8ntool/execution.go to fix lint error

@sudeepdino008
Copy link
Contributor Author

Thank you! Can you please remove function rlpHash in the file cmd/evm/internal/t8ntool/execution.go to fix lint error

Done!

@AlexeyAkhunov
Copy link
Contributor

Great work! Will need to give it a bit more review, especially core/blockchain.go and then merge. Thank you!

AlexeyAkhunov
AlexeyAkhunov reviewed Jun 24, 2022
View reviewed changes
core/blockchain.go Outdated Show resolved Hide resolved
@sudeepdino008 sudeepdino008 marked this pull request as ready for review June 27, 2022 09:32
AlexeyAkhunov
AlexeyAkhunov reviewed Jun 27, 2022
View reviewed changes
core/blockchain.go Outdated Show resolved Hide resolved
@AlexeyAkhunov AlexeyAkhunov merged commit db93d2e into ledgerwatch:devel Jul 2, 2022
@AlexeyAkhunov
Copy link
Contributor

Looks like merging this has broken some integration tests, will need to dig into this, but this is what I see for example:

go test ./tests --run TestBlockchain/TransitionTests/bcHomesteadToDao --tags integration

[EROR] [07-02|22:16:20.823] [4/16 Bodies] Uncle verification failed  number=7 hash=0x418435e9096986a6ba2a625e2a8610f74f1e16a530c6948c3d508a1bc071182d err="bad DAO pro-fork extra-data"
[EROR] [07-02|22:16:20.826] [4/16 Bodies] Uncle verification failed  number=8 hash=0xa3d7bebffb7e945595c9af5cee45faa8e0e353f28935fbffd0dc085f22e739a1 err="bad DAO pro-fork extra-data"
[EROR] [07-02|22:16:20.831] [4/16 Bodies] Uncle verification failed  number=10 hash=0x7313d768586c6108d211f5a007b6a43f5095e0e1147f96484af1aece5a2bb87b err="bad DAO pro-fork extra-data"
[EROR] [07-02|22:16:20.841] [4/16 Bodies] Uncle verification failed  number=15 hash=0x9d9339f527be0febb1713b38c88516010582b6bf372813bdd1aa7ddb20dacc99 err="bad DAO pro-fork extra-data"
[EROR] [07-02|22:16:20.843] [4/16 Bodies] Uncle verification failed  number=16 hash=0x58e70f91c436b2224cfafcc0c40eb0363980bbe69eb59e3389a2aa98605d1fb9 err="bad DAO pro-fork extra-data"
--- FAIL: TestBlockchain (0.01s)
    --- FAIL: TestBlockchain/TransitionTests/bcHomesteadToDao/DaoTransactions.json (0.20s)
        block_test.go:43: block #5 insertion into chain failed: did not import block 5 db1b027624fd6e966feef9445e4a50982e89390c7f9693c0e7f73cadcad03086
FAIL
FAIL	github.com/ledgerwatch/erigon/tests	0.735s

AlexeyAkhunov pushed a commit that referenced this pull request Jul 2, 2022
Revert "evm t8n tool to use ExecuteBlockEphemerally api (#4512)"
05669e2 
This reverts commit db93d2e.
AlexeyAkhunov added a commit that referenced this pull request Jul 2, 2022
@AlexeyAkhunov
Revert "evm t8n tool to use ExecuteBlockEphemerally api (#4512)" (#4610)
77dc35b 
* Revert "evm t8n tool to use ExecuteBlockEphemerally api (#4512)"

This reverts commit db93d2e.

* Fix compilation

Co-authored-by: Alex Sharp <alexsharp@Alexs-MacBook-Pro.local>
@AlexeyAkhunov
Copy link
Contributor

I have asked @awskii to re-work and reapply this change (it is very useful, thank you for making it @sudeepdino008 ), and to make sure tests pass and there is no de-optimisation. Please feel free to connect

@sudeepdino008
Copy link
Contributor Author

thanks @AlexeyAkhunov.
My bad, didn't realize there were integration tests in there. Great that it got caught this early on.

sudeepdino008 added a commit to covalenthq/erigon that referenced this pull request Jul 5, 2022
@sudeepdino008
evm t8n tool to use ExecuteBlockEphemerally api (ledgerwatch#4512)
ccd5866 
* fix to set V, R, S in legacy transaction

* fix to dump post-execution alloc for evm t8n

* close tx in evm t8n

* populate current difficulty and gas used in output result

- update the ExecutionResult to include corresponding info (like
  Difficulty/GasUsed)

* initial attempt at migrating 'evm t8n' to use ExecuteBlockEphemerally

* using ExecutionResult in ExecuteBlockEphemerally

* bypass validations and integrate with EphemeralExecResult

* fixing output of 'evm t8n'

- remaining bits are "stateRoot" in results.txt and "balance" field for one account in
  alloc.txt (for testdata=1)

* get ExecuteBlockEphemerally to accept getTracer lambda

* fix build failure

* test cases for evm t8n

* more test cases for evm t8n

* fix stateRoot computation in evm t8n

* remove reward argument, as EBE itself takes care of it

* final cleanups for migration to using ExecuteBlockEphemerally

* change EBEforBSC to match EBE

* fix linter issues

* manually revert an unwanted diff

* avoid calculating ReceiptHash twice

* linter check

* minor correction

* remove unnecessary logic in EBEforBsc
@sudeepdino008
Copy link
Contributor Author

@AlexeyAkhunov it's resolved #4642

AlexeyAkhunov pushed a commit that referenced this pull request Jul 7, 2022
@sudeepdino008
evm t8n to use ExecuteBlockEphemerally api (#4642)
e13a318 
* evm t8n tool to use ExecuteBlockEphemerally api (#4512)

* fix to set V, R, S in legacy transaction

* fix to dump post-execution alloc for evm t8n

* close tx in evm t8n

* populate current difficulty and gas used in output result

- update the ExecutionResult to include corresponding info (like
  Difficulty/GasUsed)

* initial attempt at migrating 'evm t8n' to use ExecuteBlockEphemerally

* using ExecutionResult in ExecuteBlockEphemerally

* bypass validations and integrate with EphemeralExecResult

* fixing output of 'evm t8n'

- remaining bits are "stateRoot" in results.txt and "balance" field for one account in
  alloc.txt (for testdata=1)

* get ExecuteBlockEphemerally to accept getTracer lambda

* fix build failure

* test cases for evm t8n

* more test cases for evm t8n

* fix stateRoot computation in evm t8n

* remove reward argument, as EBE itself takes care of it

* final cleanups for migration to using ExecuteBlockEphemerally

* change EBEforBSC to match EBE

* fix linter issues

* manually revert an unwanted diff

* avoid calculating ReceiptHash twice

* linter check

* minor correction

* remove unnecessary logic in EBEforBsc

* fix integration tests

* fix build
@sudeepdino008 sudeepdino008 deleted the evm_t8n_2 branch July 7, 2022 17:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AlexeyAkhunov AlexeyAkhunov Awaiting requested review from AlexeyAkhunov

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@sudeepdino008 @AlexeyAkhunov