fix(bridge): add mint and burn replay protection (#6) - #63
Conversation
📝 WalkthroughWalkthroughThe WPI token contract now requires redemption IDs for burns, records processed redemptions, and emits IDs in burn events. The generic mint entrypoint is removed. Relayer parsing, persistence, tests, and documentation now use redemption IDs for deduplication. ChangesRedemption replay protection
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant SorobanWpiContractClient
participant WpiToken
participant RedemptionWatcher
participant RedemptionStore
SorobanWpiContractClient->>WpiToken: Read redemption_burned event
WpiToken-->>SorobanWpiContractClient: redemptionId from topic and nonce from value
SorobanWpiContractClient-->>RedemptionWatcher: BurnEvent with redemptionId
RedemptionWatcher->>RedemptionStore: Check redemptionId
RedemptionWatcher->>RedemptionStore: Upsert unseen redemption
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/design/on-chain-reserve-oracle.md`:
- Around line 98-103: Update the documented mint_from_deposit API to remove the
admin parameter, use the contract argument order (to, amount, pi_deposit_id),
and return Result<bool, Error>. Ensure the documentation states that callers
must handle Ok(false) for volume-limit rejections.
In `@relayer/src/stellar/redemptionWatcher.ts`:
- Around line 29-31: Update the redemption deduplication flow around
hasRedemption and upsertRedemption to remain compatible with records keyed by
the legacy RPC eventId. Before processing a replay, migrate the existing record
to redemptionId or perform a legacy eventId lookup and treat it as already
processed; ensure the migrated/compatibility path prevents duplicate redemption
release while preserving the new redemptionId key for future records.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 834921a6-8d03-49cc-a8a8-b22445a31b0c
📒 Files selected for processing (8)
Stellar-contracts-v1/wpi-token/src/lib.rsStellar-contracts-v1/wpi-token/src/test.rsdocs/design/on-chain-reserve-oracle.mdrelayer/src/stellar/redemptionWatcher.tsrelayer/src/stellar/sorobanWpiContractClient.tsrelayer/src/stellar/wpiContractClient.tsrelayer/src/types.tsrelayer/test/redemptionWatcher.test.ts
| fn mint_from_deposit( | ||
| admin: Address, | ||
| to: Address, | ||
| amount: i128, | ||
| pi_deposit_id: BytesN<32>, | ||
| ) -> Result<(), Error> { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Match the documented mint API to the contract.
mint_from_deposit is minter-authorized, takes (to, amount, pi_deposit_id), and returns Result<bool, Error> so callers handle Ok(false) volume-limit rejections. The documented admin parameter and Result<(), Error> will mislead the follow-up implementation.
Proposed fix
fn mint_from_deposit(
- admin: Address,
to: Address,
amount: i128,
pi_deposit_id: BytesN<32>,
-) -> Result<(), Error> {
- // existing admin auth...
+) -> Result<bool, Error> {
+ // existing minter auth...📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fn mint_from_deposit( | |
| admin: Address, | |
| to: Address, | |
| amount: i128, | |
| pi_deposit_id: BytesN<32>, | |
| ) -> Result<(), Error> { | |
| fn mint_from_deposit( | |
| to: Address, | |
| amount: i128, | |
| pi_deposit_id: BytesN<32>, | |
| ) -> Result<bool, Error> { |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/design/on-chain-reserve-oracle.md` around lines 98 - 103, Update the
documented mint_from_deposit API to remove the admin parameter, use the contract
argument order (to, amount, pi_deposit_id), and return Result<bool, Error>.
Ensure the documentation states that callers must handle Ok(false) for
volume-limit rejections.
| if (this.store.hasRedemption(event.redemptionId)) continue; | ||
| this.store.upsertRedemption({ | ||
| redemptionId: event.eventId, | ||
| redemptionId: event.redemptionId, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Migrate legacy event-ID records before switching the dedupe key.
Records persisted by the previous watcher are keyed by RPC eventId. If an already-processed event is replayed after rollout, hasRedemption(event.redemptionId) misses that record, requeues it, and can release Pi twice. Migrate existing records or perform a legacy-key lookup during the transition.
#!/bin/bash
# Map the idempotency-store surface before inspecting persistence-key usage.
ast-grep outline relayer/src --items all --type class,interface,function --match 'Idempotency|Store'
# Locate the concrete storage implementation and any migration/compatibility logic.
rg -n -C 4 --glob '*.ts' \
'hasRedemption\s*\(|upsertRedemption\s*\(|listRedemptionsByStatus\s*\(|eventId|redemptionId' \
relayer/src🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@relayer/src/stellar/redemptionWatcher.ts` around lines 29 - 31, Update the
redemption deduplication flow around hasRedemption and upsertRedemption to
remain compatible with records keyed by the legacy RPC eventId. Before
processing a replay, migrate the existing record to redemptionId or perform a
legacy eventId lookup and treat it as already processed; ensure the
migrated/compatibility path prevents duplicate redemption release while
preserving the new redemptionId key for future records.
2 participants
Closes #6
Summary
This PR adds replay protection to the bridge flow so a Pi deposit cannot be minted twice on-chain, and a burn/redemption cannot be released twice off-chain.
Changes
mint_from_deposit(...)as the bridge mint entrypoint.DepositAlreadyProcessed.RedemptionAlreadyProcessed.RedemptionBurnedevent to carry the on-chain redemption ID alongside the nonce.Why
The previous bridge flow did not fully bind mint and burn actions to unique on-chain identifiers. That left room for double-processing if the relayer retried or replayed the same action. This change makes both directions explicit and idempotent at the contract boundary.
Testing
cargo test -p wpi-token --target-dir C:\tmp\cargo-targetNotes
Summary by CodeRabbit
New Features
Bug Fixes
Documentation