Handle PayerReportAlreadySubmitted

[fbac]

Add handling for PayerReportAlreadySubmitted by mapping revert code 0x93105c68 to ErrPayerReportAlreadySubmitted and updating payerreport.workers.SubmitterWorker.SubmitReports to mark reports submitted

This change introduces a new protocol error ErrPayerReportAlreadySubmitted with selector 0x93105c68, adds detection methods to the protocol error interfaces and blockchain error type, and updates the payer report submitter to treat this condition as a successful submission. It also extends tests to cover the new and existing revert cases and adjusts expectations accordingly.

• Add ErrPayerReportAlreadySubmitted and map selector 0x93105c68 in the protocol errors dictionary in errors.go
• Extend ProtocolError with IsErrPayerReportAlreadySubmitted and implement blockchain.BlockchainError.IsErrPayerReportAlreadySubmitted in errors.go
• Update payerreport.workers.SubmitterWorker.SubmitReports to log and mark reports submitted when IsErrPayerReportAlreadySubmitted is detected in submitter.go
• Expand test matrix to include PayerReportAlreadySubmitted, InvalidStartSequenceID, and InvalidSequenceIDs, and adjust expectations in submitter_test.go

📍Where to Start

Start with the handling in payerreport.workers.SubmitterWorker.SubmitReports in submitter.go, then review the new error mapping and detection in errors.go.

Changes since #1246 opened

• Added in-code documentation to payerreport.SubmitterWorker.SubmitReports worker method [21a5ded]

---

📊 Macroscope summarized 21a5ded. 2 files reviewed, 8 issues evaluated, 8 issues filtered, 0 comments posted

🗂️ Filtered Issues

pkg/blockchain/errors.go — 0 comments posted, 4 evaluated, 4 filtered

• line 135: protocolErrorsDictionary stores all keys in lowercase (e.g., "0x93105c68"), while the consumer tryExtractProtocolError uses a case-insensitive regex to extract the code and then performs a direct map lookup without normalizing the matched string. If the input error contains an uppercase hex sequence like 0x93105C68, the regex will match it, but the direct lookup with uppercase letters will fail, causing ErrCodeNotInDic to be returned even though the code is logically present. The fix is to normalize the matched code to a consistent case (e.g., strings.ToLower) before lookup or to store both cases in the map. [ Low confidence ]
• line 135: protocolErrorsDictionary stores all keys in lowercase (e.g., "0x93105c68"), while the consumer tryExtractProtocolError uses a case-insensitive regex to extract the code and then performs a direct map lookup without normalizing the matched string. If the input error contains an uppercase hex sequence like 0x93105C68, the regex will match it, but the direct lookup with uppercase letters will fail, causing ErrCodeNotInDic to be returned even though the code is logically present. The fix is to normalize the matched code to a consistent case (e.g., strings.ToLower) before lookup or to store both cases in the map. [ Low confidence ]
• line 135: protocolErrorsDictionary stores all keys in lowercase (e.g., "0x93105c68"), while the consumer tryExtractProtocolError uses a case-insensitive regex to extract the code and then performs a direct map lookup without normalizing the matched string. If the input error contains an uppercase hex sequence like 0x93105C68, the regex will match it, but the direct lookup with uppercase letters will fail, causing ErrCodeNotInDic to be returned even though the code is logically present. The fix is to normalize the matched code to a consistent case (e.g., strings.ToLower) before lookup or to store both cases in the map. [ Low confidence ]
• line 135: protocolErrorsDictionary stores all keys in lowercase (e.g., "0x93105c68"), while the consumer tryExtractProtocolError uses a case-insensitive regex to extract the code and then performs a direct map lookup without normalizing the matched string. If the input error contains an uppercase hex sequence like 0x93105C68, the regex will match it, but the direct lookup with uppercase letters will fail, causing ErrCodeNotInDic to be returned even though the code is logically present. The fix is to normalize the matched code to a consistent case (e.g., strings.ToLower) before lookup or to store both cases in the map. [ Low confidence ]

pkg/payerreport/workers/submitter.go — 0 comments posted, 4 evaluated, 4 filtered

• line 112: The code determines whether to submit a report based solely on the count of report.AttestationSignatures compared to a majority threshold derived from len(report.ActiveNodeIDs), without enforcing uniqueness or validating that those signatures correspond to distinct active approvers. This can allow duplicated or non-distinct signatures (or signatures from non-active nodes) to satisfy the threshold, incorrectly submitting a report without a true majority approval. Specifically, the logic at requiredAttestations := (len(report.ActiveNodeIDs) / 2) + 1 and if len(report.AttestationSignatures) < requiredAttestations { continue } fails to: (a) ensure signatures map to unique approver identities, (b) verify that approvers are members of ActiveNodeIDs, and (c) prevent duplicates. This violates the majority approval invariant and can result in on-chain submission under insufficient or invalid attestation. [ Low confidence ]
• line 127: Error propagation is inconsistent across branches: in the IsErrPayerReportAlreadySubmitted() branch, failure of SetReportSubmitted assigns latestErr = err, but in the successful submitErr == nil branch, failure of SetReportSubmitted is only logged as a warning and not recorded in latestErr. This asymmetry can lead to SubmitReports returning nil even when a state update failed in the success path, while similar failures in another branch are surfaced. Such inconsistency can surprise callers and complicate monitoring. [ Low confidence ]
• line 173: The inline comment states, "SetReportSubmitted should be able to handle that" regarding the race where the indexer records the submission before the chain confirmation, but the store implementation shown (SetReportSubmitted calling setReportSubmissionStatus with allowed-from []SubmissionStatus{SubmissionPending} only) contradicts this. If the report is already in SubmissionSubmitted (e.g., set by the indexer), calling SetReportSubmitted will likely fail due to the status transition guard. In the successful submitErr == nil path, this failure is merely warned and ignored, which directly contradicts the comment asserting that the function should handle the race. This documentation vs implementation mismatch creates uncertainty and can mask state update failures. [ Low confidence ]
• line 176: After a successful on-chain submission (i.e., submitErr == nil), the code calls w.payerReportStore.SetReportSubmitted(ctx, report.ID) and only logs a warning when it fails, without updating latestErr or otherwise signaling the failure to the caller. This silently swallows a state update failure and causes SubmitReports to return nil even when the store did not persist the Submitted status. This is inconsistent with the handling in the IsErrPayerReportAlreadySubmitted() branch, where a failure to set the submitted status assigns latestErr = err. The silent swallow at success path means the external caller may think submission completed cleanly while the database state remains stale, breaking contract parity for error propagation. [ Low confidence ]

[graphite-app]

How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

• Queue - adds this PR to the back of the merge queue
• Hotfix - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

[mkysel]

would you add some comment here explaining the reason why it might already be submitted?

[fbac]

Sure, adding them!