Skip to content

Commit c43dfcb

Browse files
committed
Improvements:
- OCR3: Fix an issue that could lead to unhealthy oracles crashing due to accessing already reaped rounds - Documentation & logging improvements Based on d4cbd50eacfac4fb9a44d9ac6132852870a722a4
1 parent c907169 commit c43dfcb

File tree

3 files changed

+38
-9
lines changed

3 files changed

+38
-9
lines changed

offchainreporting2plus/confighelper/confighelper.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import (
1515
"github.com/smartcontractkit/libocr/offchainreporting2plus/types"
1616
)
1717

18-
// OracleIdentity is identical to the internal type in package config.
18+
// OracleIdentity is identical to the internal type [config.OracleIdentity].
1919
// We intentionally make a copy to make potential future internal modifications easier.
2020
type OracleIdentity struct {
2121
OffchainPublicKey types.OffchainPublicKey
@@ -25,8 +25,9 @@ type OracleIdentity struct {
2525
TransmitAccount types.Account
2626
}
2727

28-
// PublicConfig is identical to the internal type in package config.
29-
// We intentionally make a copy to make potential future internal modifications easier.
28+
// PublicConfig is identical to the internal type [ocr2config.PublicConfig]. See
29+
// the documentation there for details. We intentionally duplicate the internal
30+
// type to make potential future internal modifications easier.
3031
type PublicConfig struct {
3132
DeltaProgress time.Duration
3233
DeltaResend time.Duration
@@ -98,6 +99,7 @@ type OracleIdentityExtra struct {
9899

99100
// ContractSetConfigArgsForIntegrationTest generates setConfig args for integration tests in core.
100101
// Only use this for testing, *not* for production.
102+
// See [ocr2config.PublicConfig] for documentation of the arguments.
101103
func ContractSetConfigArgsForEthereumIntegrationTest(
102104
oracles []OracleIdentityExtra,
103105
f int,
@@ -165,6 +167,7 @@ func ContractSetConfigArgsForEthereumIntegrationTest(
165167

166168
// ContractSetConfigArgsForTestsWithAuxiliaryArgs generates setConfig args from
167169
// the relevant parameters. Only use this for testing, *not* for production.
170+
// See [ocr2config.PublicConfig] for documentation of the arguments.
168171
func ContractSetConfigArgsForTestsWithAuxiliaryArgs(
169172
deltaProgress time.Duration,
170173
deltaResend time.Duration,
@@ -252,6 +255,7 @@ func (a AuxiliaryArgs) rng() io.Reader {
252255

253256
// ContractSetConfigArgsForTests generates setConfig args from the relevant
254257
// parameters. Only use this for testing, *not* for production.
258+
// See [ocr2config.PublicConfig] for documentation of the arguments.
255259
func ContractSetConfigArgsForTests(
256260
deltaProgress time.Duration,
257261
deltaResend time.Duration,

offchainreporting2plus/internal/ocr3/protocol/report_attestation.go

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,15 @@ func (repatt *reportAttestationState[RI]) messageReportSignatures(
172172
}
173173

174174
func (repatt *reportAttestationState[RI]) eventMissingOutcome(ev EventMissingOutcome[RI]) {
175+
if repatt.rounds[ev.SeqNr] == nil {
176+
repatt.logger.Debug("dropping EventMissingOutcome for unknown seqNr", commontypes.LogFields{
177+
"evSeqNr": ev.SeqNr,
178+
"highWaterMark": repatt.highWaterMark,
179+
"expiryRounds": repatt.expiryRounds(),
180+
})
181+
return
182+
}
183+
175184
if repatt.rounds[ev.SeqNr].verifiedCertifiedCommit != nil {
176185
repatt.logger.Debug("dropping EventMissingOutcome, already have Outcome", commontypes.LogFields{
177186
"evSeqNr": ev.SeqNr,
@@ -183,7 +192,17 @@ func (repatt *reportAttestationState[RI]) eventMissingOutcome(ev EventMissingOut
183192
}
184193

185194
func (repatt *reportAttestationState[RI]) messageCertifiedCommitRequest(msg MessageCertifiedCommitRequest[RI], sender commontypes.OracleID) {
186-
if repatt.rounds[msg.SeqNr] == nil || repatt.rounds[msg.SeqNr].verifiedCertifiedCommit == nil {
195+
if repatt.rounds[msg.SeqNr] == nil {
196+
repatt.logger.Debug("dropping MessageCertifiedCommitRequest for unknown seqNr", commontypes.LogFields{
197+
"msgSeqNr": msg.SeqNr,
198+
"sender": sender,
199+
"highWaterMark": repatt.highWaterMark,
200+
"expiryRounds": repatt.expiryRounds(),
201+
})
202+
return
203+
}
204+
205+
if repatt.rounds[msg.SeqNr].verifiedCertifiedCommit == nil {
187206
repatt.logger.Debug("dropping MessageCertifiedCommitRequest for outcome with unknown certified commit", commontypes.LogFields{
188207
"msgSeqNr": msg.SeqNr,
189208
"sender": sender,
@@ -211,8 +230,10 @@ func (repatt *reportAttestationState[RI]) messageCertifiedCommitRequest(msg Mess
211230
func (repatt *reportAttestationState[RI]) messageCertifiedCommit(msg MessageCertifiedCommit[RI], sender commontypes.OracleID) {
212231
if repatt.rounds[msg.CertifiedCommit.SeqNr] == nil {
213232
repatt.logger.Warn("dropping MessageCertifiedCommit for unknown seqNr", commontypes.LogFields{
214-
"msgSeqNr": msg.CertifiedCommit.SeqNr,
215-
"sender": sender,
233+
"msgSeqNr": msg.CertifiedCommit.SeqNr,
234+
"sender": sender,
235+
"highWaterMark": repatt.highWaterMark,
236+
"expiryRounds": repatt.expiryRounds(),
216237
})
217238
return
218239
}
@@ -507,7 +528,7 @@ func (repatt *reportAttestationState[RI]) backgroundComputeReports(ctx context.C
507528

508529
func (repatt *reportAttestationState[RI]) eventComputedReports(ev EventComputedReports[RI]) {
509530
if repatt.rounds[ev.SeqNr] == nil {
510-
repatt.logger.Debug("dropping EventComputedReports from old round", commontypes.LogFields{
531+
repatt.logger.Debug("dropping EventComputedReports for unknown seqNr", commontypes.LogFields{
511532
"evSeqNr": ev.SeqNr,
512533
"highWaterMark": repatt.highWaterMark,
513534
"expiryRounds": repatt.expiryRounds(),

offchainreporting2plus/ocr3confighelper/ocr3confighelper.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,9 @@ import (
1111
"golang.org/x/crypto/curve25519"
1212
)
1313

14-
// PublicConfig is identical to the internal type in package config.
15-
// We intentionally make a copy to make potential future internal modifications easier.
14+
// PublicConfig is identical to the internal type [ocr3config.PublicConfig]. See
15+
// the documentation there for details. We intentionally duplicate the internal
16+
// type to make potential future internal modifications easier.
1617
type PublicConfig struct {
1718
DeltaProgress time.Duration
1819
DeltaResend time.Duration
@@ -81,6 +82,7 @@ func PublicConfigFromContractConfig(skipResourceExhaustionChecks bool, change ty
8182

8283
// ContractSetConfigArgsForTestsWithAuxiliaryArgsMercuryV02 generates setConfig
8384
// args for mercury v0.2. Only use this for testing, *not* for production.
85+
// See [ocr3config.PublicConfig] for documentation of the arguments.
8486
func ContractSetConfigArgsForTestsMercuryV02(
8587
deltaProgress time.Duration,
8688
deltaResend time.Duration,
@@ -130,6 +132,7 @@ func ContractSetConfigArgsForTestsMercuryV02(
130132

131133
// ContractSetConfigArgsForTestsOCR3 generates setConfig args for OCR3. Only use
132134
// this for testing, *not* for production.
135+
// See [ocr3config.PublicConfig] for documentation of the arguments.
133136
func ContractSetConfigArgsForTests(
134137
deltaProgress time.Duration,
135138
deltaResend time.Duration,
@@ -196,6 +199,7 @@ func ContractSetConfigArgsForTests(
196199
// This function may be used in production. If you use this as part of multisig
197200
// tooling, make sure that the input parameters are identical across all
198201
// signers.
202+
// See [ocr3config.PublicConfig] for documentation of the arguments.
199203
func ContractSetConfigArgsDeterministic(
200204
// The ephemeral secret key used to encrypt the shared secret
201205
ephemeralSk [curve25519.ScalarSize]byte,

0 commit comments

Comments
 (0)