Skip to content
This repository was archived by the owner on Feb 26, 2024. It is now read-only.

fix: set the earliest block tag on startup when forking#3755

Merged
MicaiahReid merged 9 commits intoConsenSys-archive:developfrom
adjisb:fix/forking_earliest
Jan 12, 2023
Merged

fix: set the earliest block tag on startup when forking#3755
MicaiahReid merged 9 commits intoConsenSys-archive:developfrom
adjisb:fix/forking_earliest

Conversation

@adjisb
Copy link
Copy Markdown
Contributor

@adjisb adjisb commented Oct 3, 2022

This change updates Ganache's startup procedure when forking to retrieve the earliest block from the remote and cache this block as the earliest block in Ganache's block manager. This fixes a bug where calling eth_getBlockByNumber with the "earliest" block tag parameter yielded no result.

@adjisb adjisb requested a review from davidmurdoch as a code owner October 3, 2022 21:37
Copy link
Copy Markdown
Contributor

@MicaiahReid MicaiahReid left a comment

Choose a reason for hiding this comment

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

Great stuff, thanks for the PR! Just a tiny change and a question from me. Would love to see what @davidmurdoch thinks.

Comment thread src/chains/ethereum/ethereum/tests/forking/block.test.ts Outdated
Comment thread src/chains/ethereum/ethereum/src/data-managers/block-manager.ts Outdated
Copy link
Copy Markdown
Contributor

@MicaiahReid MicaiahReid left a comment

Choose a reason for hiding this comment

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

Also, we use prettier for our linting. I just ran prettier on the files you changed and added some suggested changes accordingly.

Comment thread src/chains/ethereum/ethereum/src/data-managers/block-manager.ts Outdated
Comment thread src/chains/ethereum/ethereum/src/blockchain.ts Outdated
@adjisb
Copy link
Copy Markdown
Contributor Author

adjisb commented Oct 6, 2022

Great stuff, thanks for the PR! Just a tiny change and a question from me. Would love to see what @davidmurdoch thinks.

To be honest the important stuff is in: PR3692, this PRs are just to simplify 3692

Comment thread src/chains/ethereum/ethereum/src/data-managers/block-manager.ts
Copy link
Copy Markdown
Contributor

@davidmurdoch davidmurdoch left a comment

Choose a reason for hiding this comment

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

Crazy that this bug has been in here for so long! Thanks so much for this PR!

I've requested a couple of changes, as well as pinged others for their opinions. If you don't want to make the changes yourself let me know and we can take over. Thanks again!

Comment thread src/chains/ethereum/ethereum/src/data-managers/block-manager.ts Outdated
Comment thread src/chains/ethereum/ethereum/tests/forking/block.test.ts Outdated
Comment thread src/chains/ethereum/ethereum/src/blockchain.ts
@adjisb adjisb requested review from MicaiahReid and davidmurdoch and removed request for MicaiahReid and davidmurdoch November 9, 2022 10:35
@MicaiahReid MicaiahReid force-pushed the fix/forking_earliest branch from bd337ad to 2baa643 Compare January 5, 2023 16:02
Comment thread src/chains/ethereum/ethereum/src/blockchain.ts
@MicaiahReid MicaiahReid changed the title fix: set the right value to earliest when forking fix: set the earliest block tag on startup when forking Jan 6, 2023
@MicaiahReid MicaiahReid merged commit 7ee7d4b into ConsenSys-archive:develop Jan 12, 2023
@gitpoap-bot
Copy link
Copy Markdown

gitpoap-bot Bot commented Jan 12, 2023

Congrats, your important contribution to this open-source project has earned you a GitPOAP!

GitPOAP: 2023 Ganache Contributor:

GitPOAP: 2023 Ganache Contributor GitPOAP Badge

Head to gitpoap.io & connect your GitHub account to mint!

Learn more about GitPOAPs here.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants