From f61111e2b4fbfdab4f3c96de6c9df002d839a900 Mon Sep 17 00:00:00 2001 From: Kostis Karantias Date: Fri, 31 Oct 2025 20:08:25 +0200 Subject: [PATCH] Improvements: - OCR3.1 polishing Based on e4315a2e7237ce8906c8922179d805a579f880b1. Co-authored-by: kaleofduty <59616916+kaleofduty@users.noreply.github.com> Co-authored-by: stchrysa --- .../ocr3_1/protocol/outcome_generation.go | 56 ++++++++++++++----- .../ocr3_1/protocol/state_sync_block.go | 15 ++--- .../internal/shim/ocr3_1_key_value_store.go | 2 +- 3 files changed, 50 insertions(+), 23 deletions(-) diff --git a/offchainreporting2plus/internal/ocr3_1/protocol/outcome_generation.go b/offchainreporting2plus/internal/ocr3_1/protocol/outcome_generation.go index b157b4d..92557ea 100644 --- a/offchainreporting2plus/internal/ocr3_1/protocol/outcome_generation.go +++ b/offchainreporting2plus/internal/ocr3_1/protocol/outcome_generation.go @@ -553,7 +553,7 @@ func (outgen *outcomeGenerationState[RI]) tryToMoveCertAndKVStateToCommitQC(comm return } - stb, err := tx.ReadUnattestedStateTransitionBlock(commitQC.CommitSeqNr, commitQC.StateTransitionInputsDigest) + ustb, err := tx.ReadUnattestedStateTransitionBlock(commitQC.CommitSeqNr, commitQC.StateTransitionInputsDigest) if err != nil { outgen.logger.Error("error during ReadUnattestedStateTransitionBlock", commontypes.LogFields{ "commitQCSeqNr": commitQC.CommitSeqNr, @@ -561,27 +561,54 @@ func (outgen *outcomeGenerationState[RI]) tryToMoveCertAndKVStateToCommitQC(comm }) return } - if stb == nil { + if ustb == nil { outgen.logger.Debug("unattested state transition block not found, can't move kv state", commontypes.LogFields{ "commitQCSeqNr": commitQC.CommitSeqNr, }) return } - if err := outgen.isCompatibleUnattestedStateTransitionBlockSanityCheck(commitQC, *stb); err != nil { + if err := outgen.isCompatibleUnattestedStateTransitionBlockSanityCheck(commitQC, ustb); err != nil { outgen.logger.Critical("sanity check of unattested state transition block failed, very surprising!", commontypes.LogFields{ - "commitQCSeqNr": commitQC.CommitSeqNr, - "error": err, + "commitQCSeqNr": commitQC.CommitSeqNr, + "commitQC": commitQC, + "unattestedStateTransitionBlock": ustb, + "error": err, }) return } - astb := AttestedStateTransitionBlock{ - *stb, + commitQCStateTransitionBlock := StateTransitionBlock{ + commitQC.PrevHistoryDigest, + commitQC.CommitEpoch, + commitQC.CommitSeqNr, + commitQC.StateTransitionInputsDigest, + ustb.StateWriteSet, + commitQC.StateRootDigest, + commitQC.ReportsPlusPrecursorDigest, + } + + commitQCAttestedStateTransitionBlock := AttestedStateTransitionBlock{ + commitQCStateTransitionBlock, commitQC.CommitQuorumCertificate, } - // write astb - err = tx.WriteAttestedStateTransitionBlock(commitQC.CommitSeqNr, astb) + if err := commitQCAttestedStateTransitionBlock.Verify( + outgen.config.ConfigDigest, + outgen.config.OracleIdentities, + outgen.config.ByzQuorumSize(), + ); err != nil { + outgen.logger.Critical("commitQCAttestedStateTransitionBlock is invalid, very surprising!", commontypes.LogFields{ + "commitQCSeqNr": commitQC.CommitSeqNr, + "commitQC": commitQC, + "commitQCAttestedStateTransitionBlock": commitQCAttestedStateTransitionBlock, + "unattestedStateTransitionBlock": ustb, + "error": err, + }) + return + } + + // write commitQCAttestedStateTransitionBlock + err = tx.WriteAttestedStateTransitionBlock(commitQC.CommitSeqNr, commitQCAttestedStateTransitionBlock) if err != nil { outgen.logger.Error("error writing attested state transition block", commontypes.LogFields{ "commitQCSeqNr": commitQC.CommitSeqNr, @@ -591,7 +618,7 @@ func (outgen *outcomeGenerationState[RI]) tryToMoveCertAndKVStateToCommitQC(comm } // apply write set - stateRootDigest, err := tx.ApplyWriteSet(stb.StateWriteSet.Entries) + stateRootDigest, err := tx.ApplyWriteSet(commitQCStateTransitionBlock.StateWriteSet.Entries) if err != nil { outgen.logger.Error("error applying write set", commontypes.LogFields{ "commitQCSeqNr": commitQC.CommitSeqNr, @@ -600,10 +627,10 @@ func (outgen *outcomeGenerationState[RI]) tryToMoveCertAndKVStateToCommitQC(comm return } - if stateRootDigest != stb.StateRootDigest { + if stateRootDigest != commitQCStateTransitionBlock.StateRootDigest { outgen.logger.Error("state root digest mismatch from write set application", commontypes.LogFields{ "commitQCSeqNr": commitQC.CommitSeqNr, - "expected": stb.StateRootDigest, + "expected": commitQCStateTransitionBlock.StateRootDigest, "actual": stateRootDigest, }) return @@ -646,13 +673,16 @@ func (outgen *outcomeGenerationState[RI]) persistUnattestedStateTransitionBlockA return nil } -func (outgen *outcomeGenerationState[RI]) isCompatibleUnattestedStateTransitionBlockSanityCheck(commitQC *CertifiedCommit, stb StateTransitionBlock) error { +func (outgen *outcomeGenerationState[RI]) isCompatibleUnattestedStateTransitionBlockSanityCheck(commitQC *CertifiedCommit, stb *StateTransitionBlock) error { stbStateWriteSetDigest := MakeStateWriteSetDigest( outgen.config.ConfigDigest, stb.BlockSeqNr, stb.StateWriteSet.Entries, ) + if stb.PrevHistoryDigest != commitQC.PrevHistoryDigest { + return fmt.Errorf("local state transition block prev history digest does not match commitQC: expected %s but got %s", commitQC.PrevHistoryDigest, stb.PrevHistoryDigest) + } if stbStateWriteSetDigest != commitQC.StateWriteSetDigest { return fmt.Errorf("local state transition block write set digest does not match commitQC: expected %s but got %s", commitQC.StateWriteSetDigest, stbStateWriteSetDigest) } diff --git a/offchainreporting2plus/internal/ocr3_1/protocol/state_sync_block.go b/offchainreporting2plus/internal/ocr3_1/protocol/state_sync_block.go index a20d552..57b875e 100644 --- a/offchainreporting2plus/internal/ocr3_1/protocol/state_sync_block.go +++ b/offchainreporting2plus/internal/ocr3_1/protocol/state_sync_block.go @@ -82,10 +82,7 @@ func (stasy *stateSyncState[RI]) getPendingBlocksToRequest() []seqNrRange { // [lastSeqNr+1..seqNr) (exclusive) is a gap to fill for rangeStartSeqNr := lastSeqNr + 1; rangeStartSeqNr < seqNr; rangeStartSeqNr += cfgMaxBlocksPerBlockSyncResponse { - rangeEndExclSeqNr := rangeStartSeqNr + cfgMaxBlocksPerBlockSyncResponse - if rangeEndExclSeqNr > seqNr { - rangeEndExclSeqNr = seqNr - } + rangeEndExclSeqNr := min(rangeStartSeqNr+cfgMaxBlocksPerBlockSyncResponse, seqNr) pending = append(pending, seqNrRange{rangeStartSeqNr, rangeEndExclSeqNr}) } } @@ -94,11 +91,11 @@ func (stasy *stateSyncState[RI]) getPendingBlocksToRequest() []seqNrRange { }) for rangeStartSeqNr := lastSeqNr + 1; rangeStartSeqNr <= stasy.highestPersistedStateTransitionBlockSeqNr+cfgBlockSyncLookahead && rangeStartSeqNr <= stasy.highestHeardSeqNr; rangeStartSeqNr += cfgMaxBlocksPerBlockSyncResponse { - rangeEndExclSeqNr := rangeStartSeqNr + cfgMaxBlocksPerBlockSyncResponse - if rangeEndExclSeqNr > stasy.highestPersistedStateTransitionBlockSeqNr+cfgBlockSyncLookahead { - rangeEndExclSeqNr = stasy.highestPersistedStateTransitionBlockSeqNr + cfgBlockSyncLookahead - } - // no check for rangeEndExclSeqNr > stasy.highestHeardSeqNr, because there is no harm in asking for more than exists + maxRangeEndExclSeqNr := stasy.highestPersistedStateTransitionBlockSeqNr + cfgBlockSyncLookahead + 1 + rangeEndExclSeqNr := min(rangeStartSeqNr+cfgMaxBlocksPerBlockSyncResponse, maxRangeEndExclSeqNr) + // If this is the last range that is pending, we're fine with asking for + // blocks beyond highestHeardSeqNr, as long as we're not asking for more + // than cfgMaxBlocksPerBlockSyncResponse blocks. pending = append(pending, seqNrRange{rangeStartSeqNr, rangeEndExclSeqNr}) } diff --git a/offchainreporting2plus/internal/shim/ocr3_1_key_value_store.go b/offchainreporting2plus/internal/shim/ocr3_1_key_value_store.go index efa4341..37a6860 100644 --- a/offchainreporting2plus/internal/shim/ocr3_1_key_value_store.go +++ b/offchainreporting2plus/internal/shim/ocr3_1_key_value_store.go @@ -1147,7 +1147,7 @@ func initializeSchema(keyValueDatabase ocr3_1types.KeyValueDatabase) error { it := rawTx.Range(nil, nil) defer it.Close() for it.Next() { - return fmt.Errorf("database is not empty") + return fmt.Errorf("database is not empty, this database was likely created with an unsupported alpha release of libocr") } if err := it.Err(); err != nil { return fmt.Errorf("failed to ensure database is not empty, iteration error: %w", err)