Skip to content

fix(security): emit LockedAmountMonitoringEnabled on deploy and document balance conservation invariant (FINDING-002) - #26

Open
mertcano wants to merge 1 commit into
starknet-io:SN-v0.14.2from
mertcano:mertcano-patch-2
Open

mertcano wants to merge 1 commit into
starknet-io:SN-v0.14.2from
mertcano:mertcano-patch-2

Conversation

@mertcano

Copy link
Copy Markdown

Summary of Changes

Addresses FINDING-002 (Medium Severity) regarding locked amount accounting assumptions and event consistency:

  • Observability in Token Deployment:
    • Added emission of LockedAmountMonitoringEnabled { l1_token, amount: 0 } inside handle_token_deployment.
    • Ensures external monitoring bots, indexers, and security guardians receive immediate notification that locked-amount conservation is active from token genesis.
  • Invariant & Operational Documentation:
    • Documented core balance conservation invariant locked_amount == L1_locked_balance in LockedAmount struct.
    • Added operational runbook guidance in enable_locked_amount_monitoring: governance must verify that no in-flight deposit or withdrawal messages exist when utilizing the total_supply() fallback to prevent desynchronization.
  • Unit Test Coverage:
    • Added test_failed_withdraw_exceeds_locked_amount to confirm that withdrawals exceeding the monitored L1 locked pool are rejected.

Affected Files

  • packages/bridge/src/token_bridge.cairo

Verification

scarb --release build
snforge test test_failed_withdraw_exceeds_locked_amount
snforge test

…ent balance conservation invariant (FINDING-002)

### Summary of Changes
Addresses **FINDING-002** (Medium Severity) regarding locked amount accounting assumptions and event consistency:

- **Observability in Token Deployment:**
  - Added emission of `LockedAmountMonitoringEnabled { l1_token, amount: 0 }` inside `handle_token_deployment`.
  - Ensures external monitoring bots, indexers, and security guardians receive immediate notification that locked-amount conservation is active from token genesis.
- **Invariant & Operational Documentation:**
  - Documented core balance conservation invariant `locked_amount == L1_locked_balance` in `LockedAmount` struct.
  - Added operational runbook guidance in `enable_locked_amount_monitoring`: governance must verify that no in-flight deposit or withdrawal messages exist when utilizing the `total_supply()` fallback to prevent desynchronization.
- **Unit Test Coverage:**
  - Added `test_failed_withdraw_exceeds_locked_amount` to confirm that withdrawals exceeding the monitored L1 locked pool are rejected.

### Affected Files
- `packages/bridge/src/token_bridge.cairo`

### Verification
```bash
scarb --release build
snforge test test_failed_withdraw_exceeds_locked_amount
snforge test
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