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

[sovereign] Fix revert RestoreBlockIntoPools #6753

Merged
merged 25 commits into from
Feb 14, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
eeca684
FEAT: RestoreBlockIntoPools removes last cross notarized headers base…
mariusmihaic Jan 29, 2025
1c9fd9a
FEAT: Add restoreExtendedHeaderIntoPool in RestoreBlockIntoPools
mariusmihaic Jan 29, 2025
1d8a366
FEAT: Reset last cross notarized from prev header
mariusmihaic Jan 31, 2025
69b3e4f
FEAT: Try restoreExtendedHeaderIntoPool for previous header
mariusmihaic Feb 3, 2025
8299548
FEAT: Do not restore headers from previous header
mariusmihaic Feb 3, 2025
0d3b234
FEAT: Do not RemoveLastCrossNotarizedHeaders from current header
mariusmihaic Feb 3, 2025
9c0e7d4
FIX: Ext hdr hash in debug
mariusmihaic Feb 3, 2025
b9a7fba
FEAT: Try to only sort extended headers used in block
mariusmihaic Feb 3, 2025
51aaae2
FEAT: Check extended headers from within header itself
mariusmihaic Feb 3, 2025
e339b2f
FIX: On restoreExtendedHeaderIntoPool use AddHeaderInShard
mariusmihaic Feb 4, 2025
e01a637
FEAT: Try to delete extended headers by hash
mariusmihaic Feb 4, 2025
727ac7a
FIX: For iteration extended
mariusmihaic Feb 4, 2025
357dc06
FEAT: Initial test TestSovereignChainBlockProcessor_RestoreBlockIntoP…
mariusmihaic Feb 6, 2025
cc244b9
CLN: General cleanup solution
mariusmihaic Feb 6, 2025
ba1274b
CLN: Log levels
mariusmihaic Feb 6, 2025
427d3e3
Merge branch 'feat/chain-go-sdk' into MX-16488-sov-fix-revert-blocks
mariusmihaic Feb 6, 2025
4704301
Merge branch 'feat/chain-go-sdk' into MX-16488-sov-fix-revert-blocks
mariusmihaic Feb 7, 2025
47f0c2f
FIX: After merge
mariusmihaic Feb 7, 2025
2ce2469
FEAT: Cleaned refactored unit test RestoreBlockIntoPool
mariusmihaic Feb 7, 2025
caab04e
FEAT: Extend test as func with or without extended header
mariusmihaic Feb 7, 2025
ccea952
FEAT: Test RestoreBlockIntoPoolsInvalidHeaderType
mariusmihaic Feb 10, 2025
ddb8134
FEAT: RestoreBlockIntoPoolsShouldWorkWhenHeaderNotCommitted
mariusmihaic Feb 10, 2025
129bb74
Merge branch 'feat/chain-go-sdk' into MX-16488-sov-fix-revert-blocks
mariusmihaic Feb 10, 2025
8ef11d0
Merge branch 'feat/chain-go-sdk' into MX-16488-sov-fix-revert-blocks
mariusmihaic Feb 14, 2025
1d34213
Merge branch 'feat/chain-go-sdk' into MX-16488-sov-fix-revert-blocks
mariusmihaic Feb 14, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
116 changes: 109 additions & 7 deletions process/block/sovereignChainBlock.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ var rootHash = "uncomputed root hash"
type extendedShardHeaderTrackHandler interface {
ComputeLongestExtendedShardChainFromLastNotarized() ([]data.HeaderHandler, [][]byte, error)
IsGenesisLastCrossNotarizedHeader() bool
RemoveLastCrossNotarizedHeaders()
RemoveLastSelfNotarizedHeaders()
}

type extendedShardHeaderRequestHandler interface {
Expand Down Expand Up @@ -902,7 +904,9 @@ func (scbp *sovereignChainBlockProcessor) ProcessBlock(headerHandler data.Header
}

// checkExtendedShardHeadersValidity checks if used extended shard headers are valid as construction
func (scbp *sovereignChainBlockProcessor) checkExtendedShardHeadersValidity(sovereignChainHeader data.SovereignChainHeaderHandler) error {
func (scbp *sovereignChainBlockProcessor) checkExtendedShardHeadersValidity(
sovChainHeader data.SovereignChainHeaderHandler,
) error {
lastCrossNotarizedHeader, _, err := scbp.blockTracker.GetLastCrossNotarizedHeader(core.MainChainShardId)
if err != nil {
return err
Expand All @@ -913,13 +917,17 @@ func (scbp *sovereignChainBlockProcessor) checkExtendedShardHeadersValidity(sove
"lastCrossNotarizedHeader round", lastCrossNotarizedHeader.GetRound(),
)

extendedShardHdrs := scbp.sortExtendedShardHeadersForCurrentBlockByNonce()
extendedShardHdrs, err := scbp.sortExtendedShardHeadersForCurrentBlockByNonce(sovChainHeader)
if err != nil {
return err
}

if len(extendedShardHdrs) == 0 {
return nil
}

// we should not have an epoch start block with main chain headers to be processed
if sovereignChainHeader.IsStartOfEpochBlock() {
if sovChainHeader.IsStartOfEpochBlock() {
return errors.ErrReceivedSovereignEpochStartBlockWithExtendedHeaders
}

Expand All @@ -943,8 +951,15 @@ func (scbp *sovereignChainBlockProcessor) checkExtendedShardHeadersValidity(sove
"extendedShardHeader nonce", extendedShardHdr.GetNonce(),
"extendedShardHeader round", extendedShardHdr.GetRound(),
)

err = scbp.headerValidator.IsHeaderConstructionValid(extendedShardHdr, lastCrossNotarizedHeader)
if err != nil {
log.Error("sovereignChainBlockProcessor.checkExtendedShardHeadersValidity", "error", err,
"extendedShardHdr.Nonce", extendedShardHdr.GetNonce(),
"lastCrossNotarizedHeader.Nonce", lastCrossNotarizedHeader.GetNonce(),
"extendedShardHdr.Round", extendedShardHdr.GetRound(),
"lastCrossNotarizedHeader.Round", lastCrossNotarizedHeader.GetRound(),
)
return fmt.Errorf("%w : checkExtendedShardHeadersValidity -> isHdrConstructionValid", err)
}

Expand Down Expand Up @@ -1173,20 +1188,35 @@ func (scbp *sovereignChainBlockProcessor) getExtraMissingNoncesToRequest(_ data.
return make([]uint64, 0)
}

func (scbp *sovereignChainBlockProcessor) sortExtendedShardHeadersForCurrentBlockByNonce() []data.HeaderHandler {
func (scbp *sovereignChainBlockProcessor) sortExtendedShardHeadersForCurrentBlockByNonce(
sovChainHeader data.SovereignChainHeaderHandler,
) ([]data.HeaderHandler, error) {
hdrsForCurrentBlock := make([]data.HeaderHandler, 0)

scbp.hdrsForCurrBlock.mutHdrsForBlock.RLock()
for _, headerInfo := range scbp.hdrsForCurrBlock.hdrHashAndInfo {
for _, extendedShardHeaderHash := range sovChainHeader.GetExtendedShardHeaderHashes() {
headerInfo, found := scbp.hdrsForCurrBlock.hdrHashAndInfo[string(extendedShardHeaderHash)]
if !found {
scbp.hdrsForCurrBlock.mutHdrsForBlock.RUnlock()
return nil, fmt.Errorf("%w : in sovereignChainBlockProcessor.sortExtendedShardHeadersForCurrentBlockByNonce = %s",
process.ErrMissingHeader, extendedShardHeaderHash)
}

log.Trace("sovereignChainBlockProcessor.sortExtendedShardHeadersForCurrentBlockByNonce",
"headerInfo.hdr.GetNonce()", headerInfo.hdr.GetNonce(),
"headerInfo.usedInBlock", headerInfo.usedInBlock,
)

if headerInfo.usedInBlock {
hdrsForCurrentBlock = append(hdrsForCurrentBlock, headerInfo.hdr)

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Might do it on another PR

}
}
scbp.hdrsForCurrBlock.mutHdrsForBlock.RUnlock()

process.SortHeadersByNonce(hdrsForCurrentBlock)

return hdrsForCurrentBlock
return hdrsForCurrentBlock, nil
}

func (scbp *sovereignChainBlockProcessor) verifyCrossShardMiniBlockDstMe(sovereignChainHeader data.SovereignChainHeaderHandler) error {
Expand Down Expand Up @@ -2002,7 +2032,79 @@ func (scbp *sovereignChainBlockProcessor) saveSovereignMetricsForCommittedBlock(
// RestoreBlockIntoPools restores block into pools
func (scbp *sovereignChainBlockProcessor) RestoreBlockIntoPools(header data.HeaderHandler, body data.BodyHandler) error {
scbp.restoreBlockBody(header, body)
scbp.blockTracker.RemoveLastNotarizedHeaders()

// TODO: MX-16507: check how/if to restore incoming scrs/cross chain txs once we have a testnet setup for it
// 1. For incoming scrs We should probably have something similar to (sp *shardProcessor) rollBackProcessedMiniBlocksInfo
Copy link
Contributor

Choose a reason for hiding this comment

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

actually we shouldn't really have any rollbacks anymore :D

Like with one shot finality. And we can aim for that as Andromeda is going to be released.

Merging Andromeda into this code, and what is once voted, it is a finished block.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's super awesome, actually, would simplify things a lot !

// 2. For outgoing operations, we should analyse how to revert operations or delay them before sending (wait for them
// to be finalized and impossible to rollback maybe?)

sovChainHdr, castOk := header.(data.SovereignChainHeaderHandler)
if !castOk {
return fmt.Errorf("%w in sovereignChainBlockProcessor.RestoreBlockIntoPools", errors.ErrWrongTypeAssertion)
}

err := scbp.restoreExtendedHeaderIntoPool(sovChainHdr.GetExtendedShardHeaderHashes())
if err != nil {
return err
}

scbp.extendedShardHeaderTracker.RemoveLastSelfNotarizedHeaders()

numOfNotarizedExtendedHeaders := len(sovChainHdr.GetExtendedShardHeaderHashes())
log.Debug("sovereignChainBlockProcessor.RestoreBlockIntoPools", "numOfNotarizedExtendedHeaders", numOfNotarizedExtendedHeaders)
if numOfNotarizedExtendedHeaders == 0 {
return nil
}

scbp.extendedShardHeaderTracker.RemoveLastCrossNotarizedHeaders()
return nil
}

func (scbp *sovereignChainBlockProcessor) restoreExtendedHeaderIntoPool(extendedShardHeaderHashes [][]byte) error {
for _, extendedHdrHash := range extendedShardHeaderHashes {
extendedHdr, errNotCritical := process.GetExtendedShardHeaderFromStorage(extendedHdrHash, scbp.marshalizer, scbp.store)
if errNotCritical != nil {
log.Debug("extended header is not fully processed yet and not committed in ExtendedShardHeadersUnit",
"hash", extendedHdrHash)
continue
}

scbp.dataPool.Headers().AddHeaderInShard(extendedHdrHash, extendedHdr, core.MainChainShardId)

extendedHdrStorer, err := scbp.store.GetStorer(dataRetriever.ExtendedShardHeadersUnit)
if err != nil {
log.Error("unable to get storage unit",
"unit", dataRetriever.ExtendedShardHeadersUnit.String())
return err
}

err = extendedHdrStorer.Remove(extendedHdrHash)
if err != nil {
log.Error("unable to remove hash from ExtendedShardHeadersUnit",
"hash", extendedHdrHash)
return err
}

nonceToByteSlice := scbp.uint64Converter.ToByteSlice(extendedHdr.GetNonce())
extendedHdrNonceHashStorer, err := scbp.store.GetStorer(dataRetriever.ExtendedShardHeadersNonceHashDataUnit)
if err != nil {
log.Error("unable to get storage unit",
"unit", dataRetriever.ExtendedShardHeadersNonceHashDataUnit.String())
return err
}

errNotCritical = extendedHdrNonceHashStorer.Remove(nonceToByteSlice)
if errNotCritical != nil {
log.Debug("extendedHdrNonceHashStorer.Remove error not critical",
"error", errNotCritical.Error())
}

log.Debug("extended block has been restored successfully",
"round", extendedHdr.GetRound(),
"nonce", extendedHdr.GetNonce(),
"hash", extendedHdrHash)
}

return nil
}

Expand Down
Loading