-
-
Notifications
You must be signed in to change notification settings - Fork 267
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
Prepare fast state transition for forks #2303
Conversation
Code Climate has analyzed commit 8c35022 and detected 10 issues on this pull request. Here's the issue category breakdown:
View more on Code Climate. |
// verify signature | ||
if (verifyProposer) { | ||
if (!verifyProposerSignature(postState, signedBlock)) { | ||
throw new Error("Invalid block signature"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe add block slot/root here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we use detailed typed LodestarError
s in the state transition function too? To be tackled in new PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These errors (this and below) are untouched from before. I'll make an issue to revisit state transition errors for a future PR!
// verify state root | ||
if (verifyStateRoot) { | ||
if (!config.types.Root.equals(block.stateRoot, postState.tree.root)) { | ||
throw new Error("Invalid state root"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe add block stateRoot, and the expected stateRoot here
} from "../../../util"; | ||
import {FAR_FUTURE_EPOCH} from "../../../constants"; | ||
} from "../../util"; | ||
import {FAR_FUTURE_EPOCH} from "../../constants"; | ||
import { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The statusProcessEpoch
function below seems to be better to put into phase0
folder, it's fine for now through
Motivation
We want our "fast" state transition to be multi-fork aware. To that end, there should be fork-aware utilities and a fork-aware state transition function. These files should also be organized in a fork-agnostic location.
Description
phase0.fast
top-level state transition function (fastStateTransition
), fast utils (cached beacon state, etc), and signature set utils to a top-levelfast/
directory.fastStateTransition
for other-fork logic (eg: switching outprocessSlots
,processBlocks
and upgrading theBeaconState
)fast/
exports asfast
namespace.lodestar
andspec-test-runner
packagesA followup PR can update the chain/regen/etc to use
allForks.BeaconState
rather thanphase0.BeaconState
(which will remove a lot of the explicit castings toCachedBeaconState<allForks.BeaconState>