Skip to content

fix: canonicalize bridge deposit keys and add global completion dedup#1249

Open
ouicate wants to merge 5 commits into
gonka-ai:mainfrom
ouicate:fix/bridge-deposit-canonical-dedup
Open

fix: canonicalize bridge deposit keys and add global completion dedup#1249
ouicate wants to merge 5 commits into
gonka-ai:mainfrom
ouicate:fix/bridge-deposit-canonical-dedup

Conversation

@ouicate

@ouicate ouicate commented May 26, 2026

Copy link
Copy Markdown

Summary

This fixes a bridge accounting bug where the same Ethereum deposit could produce multiple independent BridgeTransaction records, and multiple mints or escrow releases, when validators submitted identical L1 events with different string representations (for example ReceiptsRoot hex casing, or leading zeros in BlockNumber / ReceiptIndex / Amount). Vote aggregation keyed off raw strings, while mint/release only guarded completion per record, not per logical deposit.

Root Cause

generateSecureBridgeTransactionKey hashed raw field strings. ContractAddress was not normalized on ingress in all paths. ReceiptsRoot, numeric fields, and OwnerAddress were not canonicalized before hashing or storage. ValidateBasic accepted leading-zero decimal strings (^[0-9]+$). Separately, handleCompletedBridgeTransaction only checked BRIDGE_PENDING on the current record; there was no chain-wide registry keyed by L1 receipt identity, so two storage keys for one deposit could each reach quorum and mint independently.

Fix

  • Add buildCanonicalBridgeTransaction to normalize all vote-aggregation fields at ingress:
    • ContractAddress and ReceiptsRoot -> lowercase
    • BlockNumber, ReceiptIndex, Amount -> canonical decimal via big.Int (no leading zeros)
    • OwnerAddress -> canonical bech32 via AccAddressFromBech32
    • OriginChain -> lowercase
  • Harden MsgBridgeExchange.ValidateBasic: require valid bech32 OwnerAddress, reject leading zeros on numeric fields, validate ReceiptsRoot as 0x + 32-byte hex.
  • Add BridgeCompletedDepositEvents KeySet keyed by canonicalBridgeDepositEventID (chainId|blockNumber|receiptIndex|contract|owner|amount, excluding ReceiptsRoot string variants).
  • Canonicalize the completed-event ID itself so legacy or split records with leading-zero numeric variants still collapse to the same global completion key.
  • In handleCompletedBridgeTransaction, skip mint/release if the canonical deposit was already completed; mark completed only after successful mint or escrow release.

Why This Closes The Vulnerability

The exploit required two conditions: validators could create distinct vote-aggregation keys for the same L1 deposit, and completion/mint was tracked only per key. This PR removes both: equivalent deposit reports collapse to one canonical BridgeTransaction identity, and even if a legacy or Byzantine split key reached quorum, the global completion registry ensures at most one mint or escrow release per canonical L1 receipt.

Test plan

  • go test ./x/inference/types/... -run 'Bridge|BridgeExchange|Canonical'
  • go test ./x/inference/keeper/... -run 'Bridge|Canonical'
  • Confirm two MsgBridgeExchange payloads differing only in ReceiptsRoot casing or leading zeros map to the same aggregation key after fix
  • Confirm second completion for the same canonical deposit is blocked at handleCompletedBridgeTransaction

Supersedes #1196 (closed because the head fork was accidentally deleted; this branch is a clean rebase onto current main).

ouicate added 2 commits May 26, 2026 11:45
…etion dedup

Normalize MsgBridgeExchange fields before vote-aggregation hashing and
record canonical L1 receipt identity before mint or escrow release so split
keys cannot double-mint the same deposit.
Normalize receipt identity fields inside the global completion key so legacy split bridge records with leading-zero numeric variants cannot bypass completion dedup.
@tcharchian tcharchian requested review from GLiberman and patimen May 30, 2026 00:18
@ouicate

ouicate commented Jun 9, 2026

Copy link
Copy Markdown
Author

Please take a look @GLiberman

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.

1 participant