Skip to content

Conversation

@ssd04
Copy link
Contributor

@ssd04 ssd04 commented Mar 17, 2025

Reasoning behind the pull request

  • Add proofs pool new operations: Upsert, IsProofInPoolEqualTo
  • Upsert proof to pool when receiving as previous proof on header, since that will be the proof that will be managed by the chain
  • check if shard data previous proof has been already added to pool

Testing procedure

  • tbd

Pre-requisites

Based on the Contributing Guidelines the PR author and the reviewers must check the following requirements are met:

  • was the PR targeted to the correct branch?
  • if this is a larger feature that probably needs more than one PR, is there a feat branch created?
  • if this is a feat branch merging, do all satellite projects have a proper tag inside go.mod?

@ssd04 ssd04 self-assigned this Mar 17, 2025
}

// AddProof will add the provided proof to the pool, if it's not already in the pool.
// It will return true if the proof the was added to the pool.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
// It will return true if the proof the was added to the pool.
// It will return true if the proof was added to the pool.

if check.IfNil(argument.ValidityAttester) {
return nil, process.ErrNilValidityAttester
}
if check.IfNil(argument.ProofsPool) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

not need, it is only forwarded to the component

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed check

Base automatically changed from fix-proofs-pool-insert to feat/andromeda-fixes March 18, 2025 15:05
@ssd04 ssd04 marked this pull request as ready for review March 18, 2025 15:10
func (pp *proofsPool) UpsertProof(
headerProof data.HeaderProofHandler,
) bool {
if check.IfNilReflect(headerProof) {
Copy link
Contributor

Choose a reason for hiding this comment

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

check.IfNil instead for proofs IsInterfaceNil was added for the HeaderProofHandler


// IsProofInPoolEqualTo will check if the provided proof is equal with the already existing proof in the pool
func (pp *proofsPool) IsProofInPoolEqualTo(headerProof data.HeaderProofHandler) bool {
if check.IfNilReflect(headerProof) {
Copy link
Contributor

Choose a reason for hiding this comment

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

check.IfNil instread

if common.ShouldBlockHavePrevProof(interceptedHdr.HeaderHandler(), hip.enableEpochsHandler, common.EquivalentMessagesFlag) {
ok = hip.proofs.AddProof(interceptedHdr.HeaderHandler().GetPreviousProof())
log.Trace("HdrInterceptorProcessor.AddProof: add previous proof", "intercepted header hash", interceptedHdr.Hash(), "added", ok)
ok = hip.proofs.UpsertProof(interceptedHdr.HeaderHandler().GetPreviousProof())
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can remove this one, and update to upsert the one in VerifyHeaderWithProof

also it could be removed also from saveProofForPreviousHeaderIfNeeded as it should have already added on verifyHeaderWithProof

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed duplicated add prev proofs

// NewInterceptedMetaHeaderDataFactory creates an instance of interceptedMetaHeaderDataFactory
func NewInterceptedMetaHeaderDataFactory(argument *ArgInterceptedDataFactory) (*interceptedMetaHeaderDataFactory, error) {
func NewInterceptedMetaHeaderDataFactory(argument *ArgInterceptedMetaHeaderFactory) (*interceptedMetaHeaderDataFactory, error) {
if argument == nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

check for not nil proofs pool?

@sstanculeanu sstanculeanu merged commit 5728f6e into feat/andromeda-fixes Mar 19, 2025
4 checks passed
@sstanculeanu sstanculeanu deleted the proofs-pool-upsert branch March 19, 2025 08:29
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.

4 participants