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

core: drop legacy receipt types #26225

Merged
merged 6 commits into from Dec 3, 2022
Merged

core: drop legacy receipt types #26225

merged 6 commits into from Dec 3, 2022

Conversation

s1na
Copy link
Contributor

@s1na s1na commented Nov 21, 2022

This PR drops the legacy receipt types, the freezer-migrate command and the startup check. The previous attempt #22852 at this failed because there were users who still had legacy receipts in their db, so it had to be reverted #23247. Since then we added a command to migrate legacy dbs #24028.

As of the last hardforks all users either must have done the migration, or used the --ignore-legacy-receipts flag which will stop working now.

holiman
holiman reviewed Nov 21, 2022
View reviewed changes
core/types/legacy.go Outdated Show resolved Hide resolved
holiman
holiman approved these changes Nov 21, 2022
View reviewed changes
Copy link
Contributor

@holiman holiman left a comment •
edited

Choose a reason for hiding this comment

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

SGTM! (haven't tested the code)

And also drop https://github.com/ethereum/go-ethereum/blob/f5ee9060ef2108b8fda4e51fada37eac0eb24464/core/types/legacy.go

@blockchaindevsh blockchaindevsh mentioned this pull request Nov 22, 2022
Merged
@rjl493456442
Copy link
Member

rjl493456442 commented Nov 22, 2022 •
edited

@s1na IIUC, if people keep using --ignore-legacy-receipts all the time, their legacy receipts may still be untouched and they can also cross these hardforks right?

For these users, they MUST
(1) re-sync with new version geth
(2) force them to migrate with old version geth, then upgrade to new version if necessary

But if they directly upgrade to new version, --ignore-legacy-receipts is not existent, Geth will not launch at all. If they remove this flag with new version, then ERROR[06-22|15:02:49.899] Invalid receipt array RLP hash=28c41d..6bb895 err="rlp: expected input list for []*types.LogForStorage, decoding into ([]*types.ReceiptForStorage)[0](types.storedReceiptRLP).Logs" can happen again.

Just try to list all the possible scenarios ensure we can handle all of them.

rjl493456442
rjl493456442 reviewed Nov 22, 2022
View reviewed changes
core/types/receipt.go Outdated Show resolved Hide resolved
rjl493456442
rjl493456442 reviewed Nov 22, 2022
View reviewed changes
core/rawdb/accessors_chain.go
@@ -672,7 +672,7 @@ func DeleteReceipts(db ethdb.KeyValueWriter, hash common.Hash, number uint64) {
type storedReceiptRLP struct {
PostStateOrStatus []byte
CumulativeGasUsed uint64
Logs []*types.LogForStorage
Logs []*types.Log
Copy link
Member

Choose a reason for hiding this comment

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

just a nitpick and you can ignore it

Is it possible to re-use the definition in core/types package soomehow?

Copy link
Member

Choose a reason for hiding this comment

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

Maybe we can mark a TODO here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you have an idea how to do it without exporting an additional type in core/types?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think it's possible to reuse the definition in core/types.

Copy link
Contributor

Choose a reason for hiding this comment

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

It would be kind of nice to remove the DecodeRLP method of receiptLogs. I think you could just rename the type storedReceiptRLP to receiptLogs. It would save a lot of reflect indirection during decoding.

rjl493456442
rjl493456442 approved these changes Nov 22, 2022
View reviewed changes
Copy link
Member

@rjl493456442 rjl493456442 left a comment

Choose a reason for hiding this comment

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

A few nitpicks, but generally lgtm.

I think we need to deploy it on some benchmark nodes, just double-check nothing broken

@holiman
Copy link
Contributor

holiman commented Nov 30, 2022 •
edited 

INFO [11-30|09:20:57.182] Starting peer-to-peer node               instance=Geth/v1.11.0-unstable-5d67eb0e-20221122/linux-amd64/go1.19.2

Nov 30 10:23:04 bench03.ethdevops.io geth INFO [11-30|09:22:52.482] Starting peer-to-peer node instance=Geth/v1.11.0-unstable-5d67eb0e-20221122/linux-amd64/go1.19.2

@holiman holiman merged commit 10347c6 into ethereum:master Dec 3, 2022
@holiman holiman added this to the 1.11.0 milestone Dec 3, 2022
Tristan-Wilson added a commit to OffchainLabs/go-ethereum that referenced this pull request Feb 28, 2023
@Tristan-Wilson
Remove refs to dropped legacy types
2c6866f 
This removes references to types removed in
ethereum/go-ethereum#26225
@Tristan-Wilson Tristan-Wilson mentioned this pull request Feb 28, 2023
Merged
13 tasks
shekhirin pushed a commit to shekhirin/go-ethereum that referenced this pull request Jun 6, 2023
@s1na @shekhirin
core: drop legacy receipt types (ethereum#26225)
611ef58 
This PR drops the legacy receipt types, the freezer-migrate command and the startup check. The previous attempt ethereum#22852 at this failed because there were users who still had legacy receipts in their db, so it had to be reverted ethereum#23247. Since then we added a command to migrate legacy dbs ethereum#24028.

As of the last hardforks all users either must have done the migration, or used the --ignore-legacy-receipts flag which will stop working now.
@trinhdn2 trinhdn2 mentioned this pull request Jul 5, 2023
Closed
14 tasks
MoonShiesty pushed a commit to MoonShiesty/go-ethereum that referenced this pull request Aug 30, 2023
@s1na @MoonShiesty
core: drop legacy receipt types (ethereum#26225)
ef205ef 
This PR drops the legacy receipt types, the freezer-migrate command and the startup check. The previous attempt ethereum#22852 at this failed because there were users who still had legacy receipts in their db, so it had to be reverted ethereum#23247. Since then we added a command to migrate legacy dbs ethereum#24028.

As of the last hardforks all users either must have done the migration, or used the --ignore-legacy-receipts flag which will stop working now.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fjl fjl fjl left review comments

@holiman holiman holiman approved these changes

@rjl493456442 rjl493456442 rjl493456442 approved these changes

@karalabe karalabe Awaiting requested review from karalabe karalabe is a code owner

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

Successfully merging this pull request may close these issues.

None yet
4 participants
@s1na @rjl493456442 @holiman @fjl