Simplex orchestration layer preliminary implementation#427
Conversation
| // Copyright (C) 2019-2025, Ava Labs, Inc. All rights reserved. | ||
| // See the file LICENSE for licensing terms. | ||
|
|
||
| package simplex |
There was a problem hiding this comment.
should the orchastration be in its own package?
There was a problem hiding this comment.
I think it makes sense to put this in the top most package, to make it clear that it is the entry point to simplex.
| // Parameter config | ||
| Epoch: epochNum, | ||
| ReplicationEnabled: true, | ||
| StartTime: time.Now(), |
There was a problem hiding this comment.
should the orchestration layer have its own time field? i don't see where we forward/call advanceTime
| MaxProposalWait: i.Config.MaxProposalWait, | ||
| MaxRebroadcastWait: i.Config.MaxRebroadcastWait, |
There was a problem hiding this comment.
i think we should change in a seperate pr these to be consistent with avalanche go
MaxNetworkDelay time.Duration `json:"maxNetworkDelay" yaml:"maxNetworkDelay"`
MaxRebroadcastWait time.Duration `json:"maxRebroadcastWait" yaml:"maxRebroadcastWait"`| ) | ||
|
|
||
|
|
||
| type Config struct { |
There was a problem hiding this comment.
maybe this type of structure is more organized? this way we can see exactly what is used for the MSM vs epoch vs shared
type Config struct {
// Shared by both the metadata state machine and the epoch.
Common CommonConfig
// Used only to build the metadata state machine.
MSM MSMConfig
// Used only to build the simplex epoch.
Epoch EpochParams
}
type CommonConfig struct {
Logger common.Logger
Signer common.Signer
SignatureAggregatorCreator common.SignatureAggregatorCreator
VM VM
}
type MSMConfig struct {
GenesisValidatorSet metadata.NodeBLSMappings
LastNonSimplexInnerBlock metadata.VMBlock
GetPChainHeightForProposing func() uint64
GetPChainHeightForVerifying func() uint64
ComputeICMEpoch metadata.ICMEpochTransition
}
type EpochParams struct {
ID common.NodeID
Sender Sender
Storage Storage
QCDeserializer common.QCDeserializer
BlockDeserializer common.BlockDeserializer
Verifier common.SignatureVerifier
MaxProposalWait time.Duration
MaxRoundWindow uint64
MaxRebroadcastWait time.Duration
FinalizeRebroadcastTimeout time.Duration
MaxWALSize int
WALCreator wal.Creator
WALs []wal.DeletableWAL
}There was a problem hiding this comment.
this could also help with the organization we talked about at Monday's consensus sync. The caller can see which interfaces it needs to implement and which purposes
| PChainProgressListener metadata.PChainProgressListener | ||
| // LastNonSimplexBlockPChainHeight is the P-chain height of the last block built by a non-Simplex proposer. | ||
| // It is used to determine the validator set of the first ever Simplex h. | ||
| LastNonSimplexBlockPChainHeight uint64 |
There was a problem hiding this comment.
dont see where this is uesd
|
|
||
| ComputeICMEpoch metadata.ICMEpochTransition | ||
| // PChainProgressListener listens for changes in the P-chain height to trigger block building or h transitions. | ||
| PChainProgressListener metadata.PChainProgressListener |
There was a problem hiding this comment.
same, this isn't used anywhere
|
|
||
| func (e *EpochAwareStorage) Index(ctx context.Context, block common.VerifiedBlock, certificate common.Finalization) error { | ||
| if block.BlockHeader().Epoch < e.Epoch { | ||
| // This is a Telock from a previous h, so we ignore it and do not index it. |
There was a problem hiding this comment.
how does this mean the block is a telock? don't telocks have the same epoch as the sealing block?
There was a problem hiding this comment.
this is a Telock from the previous epoch. We may collect a finalization on a Telock and then we first index the sealing block and then the Telocks. Once we index the sealing block, we increment our epoch, right?
We don't want to index the Telocks too, so this is a safeguard against that.
yacovm
force-pushed
the
orchestration
branch
9 times, most recently
from
ed41178 to
55576be
Compare
yacovm
changed the title
samliok
left a comment
samliok
left a comment
There was a problem hiding this comment.
leaving what i have, was not able to review as much as i hoped today
| type epochChangeSupression struct { | ||
| lock sync.RWMutex | ||
| sealingBlockSeq uint64 | ||
| active bool | ||
| } | ||
|
|
||
| func (ecs *epochChangeSupression) isSupressionActive() bool { | ||
| ecs.lock.RLock() | ||
| defer ecs.lock.RUnlock() | ||
| return ecs.active | ||
| } | ||
|
|
||
| func (ecs *epochChangeSupression) sendProhibited(seq uint64) bool { | ||
| ecs.lock.RLock() | ||
| defer ecs.lock.RUnlock() | ||
| if !ecs.active { | ||
| return false | ||
| } | ||
| return seq > ecs.sealingBlockSeq | ||
| } | ||
|
|
||
| func (ecs *epochChangeSupression) setSupression(sealingBlockSeq uint64) { | ||
| ecs.lock.Lock() | ||
| defer ecs.lock.Unlock() | ||
| ecs.sealingBlockSeq = sealingBlockSeq | ||
| ecs.active = true | ||
| } | ||
|
|
||
| func (ecs *epochChangeSupression) clearSupression() { | ||
| ecs.lock.Lock() | ||
| defer ecs.lock.Unlock() | ||
| ecs.active = false | ||
| } | ||
|
|
||
| type Communication struct { | ||
| epochChangeSupression *epochChangeSupression | ||
| nodes atomic.Value // common.Nodes | ||
| Sender | ||
| Broadcaster | ||
| } | ||
|
|
||
| func (c *Communication) SetValidators(nodes common.Nodes) { | ||
| c.nodes.Store(nodes) | ||
| } | ||
|
|
||
| func (c *Communication) Validators() common.Nodes { | ||
| nodes, ok := c.nodes.Load().(common.Nodes) | ||
| if !ok { | ||
| return nil | ||
| } | ||
| return nodes | ||
| } | ||
|
|
||
| func (c *Communication) Broadcast(msg *common.Message) { | ||
| if c.epochChangeSupression.sendProhibited(msg.Seq()) { | ||
| return | ||
| } | ||
|
|
||
| c.Broadcaster.Broadcast(msg) | ||
| } | ||
|
|
||
| func (c *Communication) Send(msg *common.Message, destination common.NodeID) { | ||
| if c.epochChangeSupression.sendProhibited(msg.Seq()) { | ||
| return | ||
| } | ||
|
|
||
| c.Sender.Send(msg, destination) | ||
| } |
There was a problem hiding this comment.
i think we should remove this comm adapter completely.
IMO the epoch should be the one filtering these messages themselves. I think it would simplify the orchestrator code a bit, remove some weirdness with the msg.Seq function, and make the epoch more explicit about how it handles future epoch messages.
There was a problem hiding this comment.
plus if we add other messages to the epoch we need to remember to properly adjust the msg.Seq which is not that intuitive. Because whether a message gets sent to the epoch is actually controlled by msg.Seq() which is not an ideal place.
For example the block digest request literally has a seq field and you would expect msg.seq to return BlockDigestRequest.seq but instead it returns 0.
There was a problem hiding this comment.
it might make sense creating the epoch.go changes in a separate pr to be merged before this one?
| // P-chain height referenced by the last block in the chain and until the provided P-chain height. | ||
| hasValidatorSetChanged func(pChainHeight uint64) (bool, NodeBLSMappings, error) | ||
| getPChainHeight func() uint64 | ||
| getPChainHeight func(ctx context.Context) (uint64, error) |
There was a problem hiding this comment.
are these changes important to the orchestrator? if not can we separate them?
There was a problem hiding this comment.
| } | ||
|
|
||
| // mockBlockBuilder is a test BlockBuilder whose BuildBlock returns a preconfigured block/error. | ||
| type mockBlockBuilder struct { |
There was a problem hiding this comment.
i think the use of the word mock will be triggering to some members of the avalanchego team
yacovm
force-pushed
the
orchestration
branch
2 times, most recently
from
1bfcbc4 to
5eeffd9
Compare
This commit contains an implementation of an orchestration layer that orchestrates
epoch transition for validators and non-validators as they move through epochs.
The orchestration layer introduces an Instance that drives the epoch lifecycle,
switching between validator consensus and non-validator block tracking as the
node's role changes across epochs.
Key pieces:
- Instance: manages the per-epoch lifecycle, ticks the active epoch or
non-validator, and handles the handoff at epoch boundaries.
- epochChangeSupression: drops outgoing messages and cancels VM operations
while an epoch change is in progress, preventing side effects mid-transition.
- EpochAwareStorage: ignores blocks from previous epochs and signals when a
new epoch is detected.
- Config: wiring for the P-chain, storage, crypto, and networking dependencies.
Signed-off-by: Yacov Manevich <yacov.manevich@avalabs.org>
Signed-off-by: Yacov Manevich <yacov.manevich@avalabs.org>
Signed-off-by: Yacov Manevich <yacov.manevich@avalabs.org>
Signed-off-by: Yacov Manevich <yacov.manevich@avalabs.org>
| ) | ||
|
|
||
| const ( | ||
| DefaultMaxSequenceWindow = 100 |
There was a problem hiding this comment.
i think this should be a similar value to DefaultMaxRoundWindow, if not the same. Mainly because validators will reject replication that have more seqs than MaxRoundWindow.
BTW, I think we should change how we process replication requests, to not drop messages that request too many sequences but rather fill up the response with at most MaxRoundWindow. Also, we should ensure this message does not exceed the maximum number of bytes AvalancheGo clients can accept.
There was a problem hiding this comment.
Yeah good catch, I removed and changed it here.
| return lastBlock, numBlocks, nil | ||
| } | ||
|
|
||
| func (i *Instance) determineValidatorOrNot(nodes common.Nodes) bool { |
There was a problem hiding this comment.
thoughts on changing this to nodes.Contains(myNodeId)?
or making the function header a bit more clear
| func (i *Instance) determineValidatorOrNot(nodes common.Nodes) bool { | |
| func (i *Instance) isMyNodeValidator(validators common.Nodes) bool { |
There was a problem hiding this comment.
Changed it to iAmValidator before seeing this comment, I think it's equivalent.
| type MessageHandler interface { | ||
| } | ||
|
|
||
| type nodeRole byte |
There was a problem hiding this comment.
this nodeRole variable is confusing. It represents the previous role of the node right? Why do we need to know this?
There was a problem hiding this comment.
We need to know this in order to know which node type to shutdown and whether to skip shutting down a non-validator in case it is still a non validator in the next epoch.
I changed the code significantly before looking at this suggestion so please let me know if you think it's still relevant.
| config, err := i.createEpochConfig() | ||
| if err != nil { | ||
| i.Config.Logger.Error("Error creating epoch config on epoch change", zap.Error(err)) | ||
| return | ||
| } | ||
|
|
||
| if config.Epoch != epochChange.epochNum { | ||
| i.Config.Logger.Error("Epoch number mismatch on epoch change", zap.Uint64("expected", epochChange.epochNum), zap.Uint64("actual", config.Epoch)) | ||
| return | ||
| } | ||
|
|
||
| if err := i.startEpoch(config); err != nil { | ||
| i.Config.Logger.Error("Error starting new epoch on epoch change", zap.Error(err)) | ||
| } |
There was a problem hiding this comment.
i think we should have one unified way of starting a validator. We have this logic here, but we have a different method startValidator
There was a problem hiding this comment.
Agreed, I changed accordingly.
| // First, figure out if I'm still a validator. | ||
| if i.determineValidatorOrNot(epochChange.validators) { | ||
| i.Config.Logger.Info("I am now a validator") | ||
| i.startValidator() |
There was a problem hiding this comment.
we are dropping the error here, shouldn't this be propagated up? Otherwise, we will just be dropping messages not knowing why we failed starting
There was a problem hiding this comment.
whoops, good catch. Made us halting the node entirely if we fail.
| cachedStorage := NewCachedStorage(i.Config.Storage) | ||
| i.cs = cachedStorage | ||
|
|
||
| lastBlock, numBlocks, err := i.lastBlock() |
| msm *metadata.StateMachine | ||
| OnEpochChange func(seq uint64, validators common.Nodes) error | ||
| Storage | ||
| Epoch uint64 | ||
| } |
There was a problem hiding this comment.
why are some of these fields exported while others aren't?
| // GetBlock retrieves the block and finalization at [seq] with the given digest. | ||
| // If the digest is nil, the block with the given sequence number is returned. | ||
| // If [seq] the block cannot be found, returns ErrBlockNotFound. | ||
| GetBlock(seq uint64) (metadata.StateMachineBlock, *common.Finalization, error) |
There was a problem hiding this comment.
can we use the same terminology as simplex.Storage? maybe we can embed simplex.Storage in this API?
type Storage interface {
simplex.Storage
// CreateWAL creates a new Write-Ahead Log (WAL).
CreateWAL() (wal.DeletableWAL, error)There was a problem hiding this comment.
why do couple the WAL with the storage anyways?
There was a problem hiding this comment.
can we use the same terminology as simplex.Storage? maybe we can embed simplex.Storage in this API?
We can't, Retrieve conflicts because the parameters are different.
./instance.go:468:12: cannot use i.cs (variable of type *CachedStorage) as Storage value in struct literal: *CachedStorage does not implement Storage (wrong type for method Retrieve)
have Retrieve(uint64, [32]byte) (common.VerifiedBlock, *common.Finalization, error)
want Retrieve(uint64) (common.VerifiedBlock, common.Finalization, error)
why do couple the WAL with the storage anyways?
Yeah, so i was trying to minimize the number of dependencies. I was too frugal to dedicate an entire interface for a single method. I can do it though
There was a problem hiding this comment.
yea i think it makes sense to separate it
There was a problem hiding this comment.
| defer i.lock.Unlock() | ||
|
|
||
| // Stop the non-validator before doing anything else, so that we don't process any more messages while we are changing epochs. | ||
| i.stopNonValidator() |
There was a problem hiding this comment.
i dont get why we need to stop and restart a non-validator for every epoch change. This actually seems like a problem during a non-validators bootstrapping phase.
Non-validators backwards hash-chain validate epochs. This process involves storing requested future sealing blocks in memory and then indexing in order. However, when we index a sealing block, we would restart the non-validator and therefore clear its cached memory of sealing blocks meaning it needs to restart the requesting phase.
The non-validator is epoch agnostic so I think if we transition from non-validator -> non-validator all we need to do is update the communication interface to return the latest validator set.
There was a problem hiding this comment.
even further, if we go from non-validator -> validator, and we are aware of a future epoch we should still be in the non-validating phase?
There was a problem hiding this comment.
Yeah, all very good callouts 👍
I was aware that the current approach isn't ideal but was in a hurry to meet the end of month deadline.
Take a look at the latest code version in the meantime. I will add a test a bit later that will check that we don't needlessly restart non-validators.
There was a problem hiding this comment.
There was a problem hiding this comment.
Signed-off-by: Yacov Manevich <yacov.manevich@avalabs.org>
Signed-off-by: Yacov Manevich <yacov.manevich@avalabs.org>
|
|
||
| type CryptoOps interface { | ||
| Sign(message []byte) ([]byte, error) | ||
| AggregateKeys(keys ...[]byte) ([]byte, error) |
There was a problem hiding this comment.
instead of redefining the functions should we just inline the interfaces from msm
| AggregateKeys(keys ...[]byte) ([]byte, error) | |
| metadata.KeyAggregator |
same with the others
common.Signer
common.SignatureVerifier
common.QCDeserializer
metadata.KeyAggregatorThere was a problem hiding this comment.
I disagree. All these methods are single method ones.
What's the benefit of embedding an interface if it doesn't reduce the volume of the interface?
If anything, this explicit definition makes it much more clear what the integrator needs to implement.
| type ParameterConfig struct { | ||
| // WalMaxEntryCount is the maximum number of entries in the write-ahead log before it is closed. | ||
| WALMaxEntryCount int | ||
| // MaxnetworkDelay is the assumed upper bound on the network delay for messages to be delivered. |
There was a problem hiding this comment.
| // MaxnetworkDelay is the assumed upper bound on the network delay for messages to be delivered. | |
| // MaxNetworkDelay is the assumed upper bound on the network delay for messages to be delivered. |
| // GetBlock retrieves the block and finalization at [seq] with the given digest. | ||
| // If the digest is nil, the block with the given sequence number is returned. | ||
| // If [seq] the block cannot be found, returns ErrBlockNotFound. | ||
| GetBlock(seq uint64) (metadata.StateMachineBlock, *common.Finalization, error) |
There was a problem hiding this comment.
yea i think it makes sense to separate it
| } | ||
| } | ||
|
|
||
| func (m *Message) Seq() uint64 { |
There was a problem hiding this comment.
wait why is this added back?
There was a problem hiding this comment.
I rebased and out of habit added it back
Signed-off-by: Yacov Manevich <yacov.manevich@avalabs.org>
…y after bootstrapping Signed-off-by: Yacov Manevich <yacov.manevich@avalabs.org>
Signed-off-by: Yacov Manevich <yacov.manevich@avalabs.org>
| // If the P-chain height changed, re-evaluate again whether we should transition to a new epoch, | ||
| // or continue waiting to build a block. | ||
| if bbd.getPChainHeight() != pChainHeight { | ||
| h := bbd.getPChainHeight() |
There was a problem hiding this comment.
any reason for this diff?
There was a problem hiding this comment.
Once we introduce a context and an error return value, we will need to do this, so the diff will be smaller then.
| ID common.NodeID | ||
| } | ||
|
|
||
| type MessageHandler interface { |
|
|
||
| func (e *EpochAwareStorage) Index(ctx context.Context, block common.VerifiedBlock, certificate common.Finalization) error { | ||
| if block.BlockHeader().Epoch < e.epoch { | ||
| // This is a Telock from a previous h, so we ignore it and do not index it. |
There was a problem hiding this comment.
telock from a previous height? or previous epoch?
| if err := e.onEpochChange(block.BlockHeader().Seq, block.SealingBlockInfo().ValidatorSet); err != nil { | ||
| return err | ||
| } | ||
| // We are now in a new h, so we update the h number to prevent indexing Telocks from the previous h. |
There was a problem hiding this comment.
can we use the term epoch instead of h? unless h is supposed to represent height, then can we rename to height
There was a problem hiding this comment.
I don't know why Goland refactored epoch to h 🤦♂️
| msm *metadata.StateMachine | ||
| lock sync.RWMutex | ||
| Storage | ||
| cache map[[32]byte]cachedBlock |
There was a problem hiding this comment.
i think we were planning on using simplex.digests instead of [32]byte? maybe for a follow up pr?
| close(i.stopCh) | ||
| } | ||
|
|
||
| if i.e != nil { |
There was a problem hiding this comment.
unnecessary since this is checked in stopValidator. Or is this defensive?
| } | ||
|
|
||
| if i.e != nil { | ||
| i.stopValidator() |
There was a problem hiding this comment.
why don't we potentially stop the non-validator as well?
| func (i *Instance) wireReplicationResponse(msg *common.Message) error { | ||
| resp := msg.ReplicationResponse | ||
| if resp.LatestRound != nil && resp.LatestRound.Block != nil { | ||
| pb, isParsedBlock := resp.LatestRound.Block.(*ParsedBlock) | ||
| if !isParsedBlock { | ||
| return fmt.Errorf("expected ParsedBlock, got %T", resp.LatestRound.Block) | ||
| } | ||
| resp.LatestRound.Block = &cachedBlock{ | ||
| cache: i.cs, | ||
| ParsedBlock: pb, | ||
| } | ||
| pb.msm = i.msm | ||
| } | ||
| if resp.LatestSeq != nil && resp.LatestSeq.Block != nil { | ||
| pb, isParsedBlock := resp.LatestSeq.Block.(*ParsedBlock) | ||
| if !isParsedBlock { | ||
| return fmt.Errorf("expected ParsedBlock, got %T", resp.LatestSeq.Block) | ||
| } | ||
| resp.LatestSeq.Block = &cachedBlock{ | ||
| cache: i.cs, | ||
| ParsedBlock: pb, | ||
| } | ||
| pb.msm = i.msm | ||
| } | ||
| for j, datum := range resp.Data { | ||
| if datum.Block == nil { | ||
| continue | ||
| } | ||
| pb, isParsedBlock := datum.Block.(*ParsedBlock) | ||
| if !isParsedBlock { | ||
| return fmt.Errorf("expected ParsedBlock, got %T", datum.Block) | ||
| } | ||
| resp.Data[j].Block = &cachedBlock{ | ||
| cache: i.cs, | ||
| ParsedBlock: pb, | ||
| } | ||
| pb.msm = i.msm | ||
| } | ||
| return nil | ||
| } |
There was a problem hiding this comment.
simplification if we implement the shared helper
| func (i *Instance) wireReplicationResponse(msg *common.Message) error { | |
| resp := msg.ReplicationResponse | |
| if resp.LatestRound != nil && resp.LatestRound.Block != nil { | |
| pb, isParsedBlock := resp.LatestRound.Block.(*ParsedBlock) | |
| if !isParsedBlock { | |
| return fmt.Errorf("expected ParsedBlock, got %T", resp.LatestRound.Block) | |
| } | |
| resp.LatestRound.Block = &cachedBlock{ | |
| cache: i.cs, | |
| ParsedBlock: pb, | |
| } | |
| pb.msm = i.msm | |
| } | |
| if resp.LatestSeq != nil && resp.LatestSeq.Block != nil { | |
| pb, isParsedBlock := resp.LatestSeq.Block.(*ParsedBlock) | |
| if !isParsedBlock { | |
| return fmt.Errorf("expected ParsedBlock, got %T", resp.LatestSeq.Block) | |
| } | |
| resp.LatestSeq.Block = &cachedBlock{ | |
| cache: i.cs, | |
| ParsedBlock: pb, | |
| } | |
| pb.msm = i.msm | |
| } | |
| for j, datum := range resp.Data { | |
| if datum.Block == nil { | |
| continue | |
| } | |
| pb, isParsedBlock := datum.Block.(*ParsedBlock) | |
| if !isParsedBlock { | |
| return fmt.Errorf("expected ParsedBlock, got %T", datum.Block) | |
| } | |
| resp.Data[j].Block = &cachedBlock{ | |
| cache: i.cs, | |
| ParsedBlock: pb, | |
| } | |
| pb.msm = i.msm | |
| } | |
| return nil | |
| } | |
| func (i *Instance) wireReplicationResponse(msg *common.Message) error { | |
| resp := msg.ReplicationResponse | |
| quorumRounds := []*common.QuorumRound{resp.LatestRound, resp.LatestSeq} | |
| for j := range resp.Data { | |
| quorumRounds = append(quorumRounds, &resp.Data[j]) | |
| } | |
| for _, qr := range quorumRounds { | |
| if qr == nil || qr.Block == nil { | |
| continue | |
| } | |
| block, err := i.cacheBlock(qr.Block) | |
| if err != nil { | |
| return err | |
| } | |
| qr.Block = block | |
| } | |
| return nil | |
| } |
| lastNonSimplexHeight := i.Config.LastNonSimplexInnerBlock.Height() | ||
| genesisValidatorSet := i.Config.PlatformChain.GenesisValidatorSet() | ||
| nodes, epochNum, err := constructEpochAndValidatorSet(lastNonSimplexHeight, genesisValidatorSet, numBlocks, ParsedBlock{StateMachineBlock: lastBlock}, i.Config.Storage) |
There was a problem hiding this comment.
nit: this is duplicated in the Start function as well, might make sense to create another shared helper?
// currentEpochState reads the last persisted block and derives the epoch
// number and validator set the chain is currently in.
func (i *Instance) currentEpochState() (metadata.StateMachineBlock, common.Nodes, uint64, error) {
lastBlock, numBlocks, err := i.lastBlock()
if err != nil {
return metadata.StateMachineBlock{}, nil, 0, err
}
lastNonSimplexHeight := i.Config.LastNonSimplexInnerBlock.Height()
genesisValidatorSet := i.Config.PlatformChain.GenesisValidatorSet()
nodes, epochNum, err := constructEpochAndValidatorSet(lastNonSimplexHeight, genesisValidatorSet, numBlocks, ParsedBlock{StateMachineBlock: lastBlock}, i.Config.Storage)
if err != nil {
return metadata.StateMachineBlock{}, nil, 0, fmt.Errorf("error determining latest epoch and validator set: %w", err)
}
return lastBlock, nodes, epochNum, nil
}There was a problem hiding this comment.
I am not sure it's worth it, because we essentially add a new function that returns a bunch of parameters and then we lose their context/purpose, and we don't actually save a lot in the number of code lines saves as it's only duplicated once.
| return nodes, epochNum, nil | ||
| } | ||
|
|
||
| func validatorSetToNodes(genesisValidatorSet metadata.NodeBLSMappings) common.Nodes { |
There was a problem hiding this comment.
| func validatorSetToNodes(genesisValidatorSet metadata.NodeBLSMappings) common.Nodes { | |
| func validatorSetToNodes(validatorSet metadata.NodeBLSMappings) common.Nodes { |
| if err := i.startEpoch(epochConfig); err != nil { | ||
| return err | ||
| } | ||
|
|
||
| return nil |
There was a problem hiding this comment.
| if err := i.startEpoch(epochConfig); err != nil { | |
| return err | |
| } | |
| return nil | |
| return i.startEpoch(epochConfig) |
| return i.startValidatorOrNonValidator(epochChange.validators, epochChange.epochNum) | ||
| } | ||
|
|
||
| func constructEpochAndValidatorSet(lastNonSimplexInnerBlockHeight uint64, genesisValidatorSet metadata.NodeBLSMappings, numBlocks uint64, lastBlock ParsedBlock, storage Storage) (common.Nodes, uint64, error) { |
There was a problem hiding this comment.
since this is an isolated function, should we have claude write some tests that we review for it?
alternatively since this is only used once should we just update the caller? I told claude to do this and it seemed to have made some good cleanups
// currentEpochState reads the last persisted block and derives the epoch
// number and validator set the chain is currently in.
func (i *Instance) currentEpochState() (metadata.StateMachineBlock, common.Nodes, uint64, error) {
lastBlock, numBlocks, err := i.lastBlock()
if err != nil {
return metadata.StateMachineBlock{}, nil, 0, err
}
parsed := ParsedBlock{StateMachineBlock: lastBlock}
sealingInfo := parsed.SealingBlockInfo()
switch {
// If all we have in the ledger is non-Simplex blocks, load the validator set from genesis.
case i.Config.LastNonSimplexInnerBlock.Height()+1 == numBlocks:
genesisValidatorSet := i.Config.PlatformChain.GenesisValidatorSet()
return lastBlock, validatorSetToNodes(genesisValidatorSet), i.Config.LastNonSimplexInnerBlock.Height() + 1, nil
// If the last block persisted is a sealing block, then we are in the next epoch,
// which is identified by the sealing block's sequence and carries its validator set.
case sealingInfo != nil:
return lastBlock, sealingInfo.ValidatorSet, parsed.BlockHeader().Seq, nil
// Else, we have at least one Simplex block in the ledger, and it's not a sealing block.
// Its Epoch field is the sequence of the sealing block that started this epoch.
default:
sealingBlockSeq := parsed.BlockHeader().Epoch
sealingBlock, _, err := i.Config.Storage.GetBlock(sealingBlockSeq)
if err != nil {
return metadata.StateMachineBlock{}, nil, 0, fmt.Errorf("error retrieving sealing block from storage: %w", err)
}
info := (&ParsedBlock{StateMachineBlock: sealingBlock}).SealingBlockInfo()
if info == nil {
return metadata.StateMachineBlock{}, nil, 0, fmt.Errorf("expected sealing block at seq %d, but got a non-sealing block", sealingBlockSeq)
}
return lastBlock, info.ValidatorSet, sealingBlockSeq, nil
}
}Signed-off-by: Yacov Manevich <yacov.manevich@avalabs.org>
2 participants
This commit contains an implementation of an orchestration layer that orchestrates
epoch transition for validators and non-validators as they move through epochs.
The orchestration layer introduces an Instance that drives the epoch lifecycle,
switching between validator consensus and non-validator block tracking as the
node's role changes across epochs.
Key pieces:
non-validator, and handles the handoff at epoch boundaries.
while an epoch change is in progress, preventing side effects mid-transition.
new epoch is detected.