-
Notifications
You must be signed in to change notification settings - Fork 107
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
Sync execution update on demand #1154
Conversation
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.
Few minor comments, but it looks great! Nice work.
if checkpointSlot > lastFinalizedHeaderState.BeaconSlot { | ||
checkpointSlot = lastFinalizedHeaderState.BeaconSlot | ||
} |
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.
Please add a comment here to explain why this check.
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.
Co-authored-by: Clara van Staden <[email protected]>
Co-authored-by: Clara van Staden <[email protected]>
err = beaconHeader.SyncExecutionHeader(ctx, *blockHeader.ParentBeaconRoot) | ||
if err == header.ErrBeaconHeaderNotFinalized { | ||
logger.Warn("beacon header not finalized, just skipped") | ||
continue | ||
} |
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.
Just scan from the last block number and wait to continue until the execution header for that event log is finalized.
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.
+1
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.
Almost there! Looks great 😄
if beaconState.BeaconSlot > slot && beaconState.BeaconSlot < slot+8192 { | ||
return beaconState, nil | ||
} | ||
return beaconState, fmt.Errorf("Can't find checkpoint on chain for slot %d", slot) |
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.
return beaconState, fmt.Errorf("Can't find checkpoint on chain for slot %d", slot) | |
return beaconState, fmt.Errorf("find checkpoint on chain for slot %d", slot) |
The error message is appended continuously, like here https://github.com/Snowfork/snowbridge/pull/1154/files#diff-92cc554d5b7218e7576839ca25a3b71ae0bbfa8200552beaf703a95a1a0da2a3R292, so simplify and lowercase the error message.
return beaconRoot, nil | ||
} | ||
|
||
func (wr *ParachainWriter) FindCheckPointBackward(slot uint64) (state.FinalizedHeader, error) { |
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.
func (wr *ParachainWriter) FindCheckPointBackward(slot uint64) (state.FinalizedHeader, error) { | |
func (wr *ParachainWriter) FindCheckPoint(slot uint64) (state.FinalizedHeader, error) { |
if beaconState.BeaconSlot < slot { | ||
break | ||
} | ||
if beaconState.BeaconSlot > slot && beaconState.BeaconSlot < slot+8192 { |
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 rather use SLOTS_PER_EPOCH * EPOCHS_PER_SYNC_COMMITTEE_PERIOD
here. Please also add code comments explaining this line and the one above, it's not clear to me what these 2 checks check.
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.
It's to backward to find check point(slot<beaconState.BeaconSlot<slot+8192)
For the first check beaconState.BeaconSlot < slot
which means there is no valid checkpoint on chain so just exit the loop.
For the second check if beaconState.BeaconSlot > slot && beaconState.BeaconSlot < slot+8192
which means we find that so exit the loop.
return checkpoint, ErrBeaconHeaderNotFinalized | ||
} | ||
if checkpointSlot < lastFinalizedHeaderState.BeaconSlot { | ||
log.WithFields(log.Fields{"calculatedCheckpointSlot": checkpointSlot, "lastFinalizedSlot": lastFinalizedHeaderState.BeaconSlot}).Info("fetch checkpoint on chain backward from history") |
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.
log.WithFields(log.Fields{"calculatedCheckpointSlot": checkpointSlot, "lastFinalizedSlot": lastFinalizedHeaderState.BeaconSlot}).Info("fetch checkpoint on chain backward from history") | |
log.WithFields(log.Fields{"calculatedCheckpointSlot": checkpointSlot, "lastFinalizedSlot": lastFinalizedHeaderState.BeaconSlot}).Info("fetch closest checkpoint on chain") |
} | ||
err = h.writer.WriteToParachainAndWatch(ctx, "EthereumBeaconClient.submit_execution_header", headerUpdate) | ||
if err != nil { | ||
return fmt.Errorf("submit_execution_header: %w", err) |
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.
return fmt.Errorf("submit_execution_header: %w", err) | |
return fmt.Errorf("submit execution header : %w", err) |
Requires: Snowfork/polkadot-sdk#123