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

Shanghai/Agra HF #8504

Merged
merged 7 commits into from
Oct 20, 2023
Merged

Shanghai/Agra HF #8504

merged 7 commits into from
Oct 20, 2023

Conversation

anshalshukla
Copy link
Collaborator

No description provided.

erigon-lib/chain/chain_config.go Outdated Show resolved Hide resolved
erigon-lib/chain/chain_config.go Outdated Show resolved Hide resolved
@0xKrishna 0xKrishna changed the title Shanghai/Agra HF Shanghai/Agra HF {WIP} Oct 18, 2023
consensus/bor/bor.go Outdated Show resolved Hide resolved
core/vm/interpreter.go Outdated Show resolved Hide resolved
@0xKrishna
Copy link
Collaborator

Left few comments, Rest LGTM.

@anshalshukla anshalshukla changed the title Shanghai/Agra HF {WIP} Shanghai/Agra HF Oct 19, 2023
@mh0lt
Copy link
Contributor

mh0lt commented Oct 19, 2023

You can't reference github.com/ledgerwatch/erigon/core/rawdb from erigon-lib code. Erigon lib needs to be self contained.

This will need to be fixed before ci will pass.

@yperbasis
Copy link
Member

@anshalshukla Why introduce AgraBlock to the code? Can't we simply use ShanghaiBlock instead?

@anshalshukla
Copy link
Collaborator Author

anshalshukla commented Oct 19, 2023

The problem is bor HF are based on block whereas ethereum now have it time based. Using ShanghaiBlock would mean we cannot use IsShanghai function as it takes in timestamp as parameter whereas bor needs a block there to figure out the fork
@yperbasis

@yperbasis
Copy link
Member

The problem is bor HF are based on block whereas ethereum now have it time based. Using ShanghaiBlock would mean we cannot use IsShanghai function as it takes in timestamp as parameter whereas bor needs a block there to figure out the fork @yperbasis

Understood, makes sense

core/genesis_write.go Outdated Show resolved Hide resolved
consensus/bor/bor.go Outdated Show resolved Hide resolved
consensus/bor/bor.go Outdated Show resolved Hide resolved
Copy link
Member

@yperbasis yperbasis left a comment

Choose a reason for hiding this comment

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

If I understand correctly, withdrawals shouldn't be present at all in Agra blocks. If that's the case, withdrawals must be nil rather than an empty array.

@anshalshukla anshalshukla merged commit 7dce126 into devel Oct 20, 2023
7 checks passed
@anshalshukla anshalshukla deleted the shanghai branch October 20, 2023 19:46
@AlexeyAkhunov
Copy link
Contributor

Causes this error on bor-mainnet node (works on the previous commit though):

[INFO] [10-20|21:03:05.822] [7/15 Execution] Blocks execution        from=48956844 to=48957092
[EROR] [10-20|21:03:05.823] Staged Sync                              err="runtime error: index out of range [-1], trace: [stageloop.go:105 panic.go:890 panic.go:113 chain_config.go:587 chain_config.go:543 state_transition.go:446 state_transition.go:185 state_processor.go:61 state_processor.go:118 blockchain.go:121 stage_execute.go:177 stage_execute.go:471 default_stages.go:120 sync.go:430 sync.go:331 stageloop.go:149 stageloop.go:71 asm_amd64.s:1598]"

@yperbasis
Copy link
Member

Causes this error on bor-mainnet node (works on the previous commit though):

[INFO] [10-20|21:03:05.822] [7/15 Execution] Blocks execution        from=48956844 to=48957092
[EROR] [10-20|21:03:05.823] Staged Sync                              err="runtime error: index out of range [-1], trace: [stageloop.go:105 panic.go:890 panic.go:113 chain_config.go:587 chain_config.go:543 state_transition.go:446 state_transition.go:185 state_processor.go:61 state_processor.go:118 blockchain.go:121 stage_execute.go:177 stage_execute.go:471 default_stages.go:120 sync.go:430 sync.go:331 stageloop.go:149 stageloop.go:71 asm_amd64.s:1598]"

Should be fixed by #8553

yperbasis added a commit that referenced this pull request Oct 22, 2023
burntContract, introduced in PR #8504, is more generic than
eip1559FeeCollector
yperbasis added a commit that referenced this pull request Oct 23, 2023
Fixes and simplifications to PR #8504
@anshalshukla anshalshukla restored the shanghai branch October 25, 2023 06:53
@AskAlexSharov AskAlexSharov deleted the shanghai branch July 5, 2024 06:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants