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

Hyperspace nv20: upgrade and migration. #10243

Merged
merged 69 commits into from
Feb 14, 2023

Conversation

raulk
Copy link
Member

@raulk raulk commented Feb 12, 2023

This PR adds support for Hyperspace's network version 20.

Depedencies-wise:

Functionality-wise:

  • Re-adds support for the legacy event type (containing no codec field).
  • Adds code to adapt old EVM events to new EVM events, so that the rest of the system only needs to know about the latter.
  • Maintains consensus on events in nv19 and below.
  • Migrates the event index by upgrading old events to new events, backfilling the codec, removing the CBOR framing from values, rewriting topic keys, and padding topic values to 32 bytes.
  • Makes all parts of the system use ChainGetEvents to load events. This makes it easy to centralise the "upgrade" logic when reading old events from the store.
  • Retains compatibility with Delegated signatures carrying MethodSend in network versions below 20.
  • On network versions below 20, still generates Delegated messages with MethodSend if no parameters are set.

Test plan:

See #10240.

Base automatically changed from raulk/hyperspace-nv20-backwards-compatibility to raulk/hyperspace-nv20-staging February 12, 2023 21:04
@raulk raulk force-pushed the raulk/hyperspace-nv20-staging branch 2 times, most recently from 1f175fe to bd63953 Compare February 12, 2023 21:52
@raulk raulk force-pushed the raulk/hyperspace-nv20-migration branch from e43a1cc to f189abf Compare February 12, 2023 21:59
Otherwise we may, e.g., try to estimate gas on a message to an f4
address before the nv18 migration.

I'm _not_ checking the "prior messages" here as this is just a sanity
check.
Copy link
Contributor

@arajasek arajasek left a comment

Choose a reason for hiding this comment

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

Broadly looks good to me -- didn't look too closely at the events changes, but everything else looks like this will be a successful upgrade :)

chain/consensus/filcns/upgrades.go Outdated Show resolved Hide resolved
chain/messagepool/messagepool.go Show resolved Hide resolved
chain/vm/fvm.go Outdated Show resolved Hide resolved
itests/migration_nv20_test.go Show resolved Hide resolved
return nil, nil
}

// Hyperspace nv20: we peek into the events array to figure out if these
Copy link
Member

Choose a reason for hiding this comment

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

This is highlighting an issue with this API. I'm not sure what the right fix is, but this is going to be painful going forward if/when we change the events structure.

@raulk raulk merged commit b115c18 into raulk/hyperspace-nv20-staging Feb 14, 2023
@raulk raulk deleted the raulk/hyperspace-nv20-migration branch February 14, 2023 16:35
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.

7 participants