Skip to content

fix: SR-126/127/128/129 — shard remittance index, CI legacy-tests, corridor guardrails, partial-payout fee - #1

Open
Good-Coded wants to merge 1 commit into
mainfrom
fix/sr-126-127-128-129
Open

fix: SR-126/127/128/129 — shard remittance index, CI legacy-tests, corridor guardrails, partial-payout fee#1
Good-Coded wants to merge 1 commit into
mainfrom
fix/sr-126-127-128-129

Conversation

@Good-Coded

@Good-Coded Good-Coded commented Aug 28, 2026

Copy link
Copy Markdown
Owner

Summary

Fixes four security/correctness issues reported against the SwiftRemit Soroban contract.

SR-126 — Cap/shard per-sender/per-agent remittance index (closes Haroldwonder#1269)

Problem: append_sender_remittance and append_agent_remittance stored the entire list of IDs under one ledger entry. Every new remittance read+rewrote the full vector O(n) with no upper bound — a high-volume agent could eventually hit Soroban's per-entry size/instruction limit, breaking create_remittance/confirm_payout for that address.

Fix:

  • Added DataKey::SenderRemittancesShard(Address, u32), SenderRemittancesCount(Address), AgentRemittancesShard(Address, u32), AgentRemittancesCount(Address).
  • REMITTANCE_INDEX_SHARD_SIZE = 500: each ledger entry holds at most 500 u64 IDs (~4 KB), well within Soroban limits.
  • append_* functions write to the current shard and bump the count; they never grow a single entry beyond the cap.
  • get_* functions reassemble shards in order. Both fall back to the legacy flat key for backward compatibility with existing on-chain state.

SR-127 — Run legacy-tests in every high-stakes CI gate (closes Haroldwonder#1270)

Problem: audit-freeze.yml, ci-consolidated.yml, and mainnet-checklist.yml ran plain cargo test, silently skipping ~26 test modules gated on --features legacy-tests. The two highest-stakes gates never exercised the bulk of the test suite.

Fix: Added cargo test --features legacy-tests as a second test step in each of the three workflows.

SR-128 — Bring create_remittance_with_corridor to parity (closes Haroldwonder#1271)

Problem: The corridor entry point was missing four protections from the primary path, letting callers bypass reputation gating, corridor caps, and analytics.

Fix: Added the missing guards:

  1. is_migration_in_progress — blocks calls during a live migration.
  2. Min-agent-reputation gate with agent_suspended event.
  3. check_and_increment_corridor_volume against the configured cap.
  4. append_agent_remittance — the agent index was never populated.
  5. increment_remittance_count — analytics counter was skipped.

SR-129 — Fix confirm_partial_payout to honor the creation-time fee (closes Haroldwonder#1272)

Problem: confirm_partial_payout called calculate_fees_with_breakdown(remittance.amount, None, None), recalculating the fee from the current global strategy at settlement time. If fee/strategy changed after creation, or the original remittance had a volume discount, the net payout would differ from what was escrowed.

Fix: Replaced with breakdown_from_platform_fee(&env, remittance.amount, remittance.fee) — deriving net payout from the stored fee, identical to confirm_payout and resolve_dispute.

Additional CI-unblocking fixes

Fix Reason
Qualify abuse_protection::check_rate_limit at 3 call sites abuse_protection::* and rate_limit::* both export the same name; Rust rejected the ambiguity. Pre-existing on main.
Switch reqwest dev-dep to rustls-tls Removes pkg-config/openssl system dep that failed cargo test in clean environments.
Add missing integrator arg to create_remittance in 4 test files Option<Address> param added in a prior PR but 4 test files not updated.

Testing

cargo build                          # clean
cargo test                           # passes
cargo test --features legacy-tests   # passes

Closes Haroldwonder#1269
Closes Haroldwonder#1270
Closes Haroldwonder#1271
Closes Haroldwonder#1272

@Good-Coded
fix: SR-126/127/128/129 — shard remittance index, add legacy-tests to…
c79ef49 
… CI, corridor guardrails parity, fix partial-payout fee

SR-126: Cap/shard per-sender/per-agent remittance index
- Add DataKey::SenderRemittancesShard(Address, u32) and AgentRemittancesShard(Address,
  u32) with a corresponding *Count key per address
- REMITTANCE_INDEX_SHARD_SIZE = 500: each ledger entry holds at most 500 IDs
  (≈4 KB), keeping every append O(shard_size) instead of O(n)
- append_sender_remittance / append_agent_remittance now write to the current
  shard and increment the count; they never grow a single entry past the cap
- get_sender_remittances / get_agent_remittances reassemble shards in order;
  both fall back to the legacy flat SenderRemittances/AgentRemittances key so
  existing on-chain data continues to work after the upgrade

SR-127: Run legacy-tests suite in every high-stakes CI gate
- audit-freeze.yml: add 'cargo test --locked --features legacy-tests' step
- ci-consolidated.yml: add 'cargo test --features legacy-tests --verbose' step
- mainnet-checklist.yml: add 'cargo test --features legacy-tests --verbose' step

SR-128: Bring create_remittance_with_corridor to parity with create_remittance
- Add is_migration_in_progress guard (blocks calls during live migration)
- Add minimum-agent-reputation gate (get_min_agent_reputation /
  compute_agent_reputation) with agent_suspended event emission
- Add check_and_increment_corridor_volume against the global/to_country cap
- Add append_agent_remittance so the agent index is populated (was missing)
- Add increment_remittance_count so analytics counter stays accurate

SR-129: Fix confirm_partial_payout to honor the fee quoted at creation time
- Replace fee_service::calculate_fees_with_breakdown(remittance.amount, None, None)
  with fee_service::breakdown_from_platform_fee(remittance.amount, remittance.fee)
- Mirrors the approach already used in confirm_payout and resolve_dispute
- Prevents over/under-payment when the platform fee or strategy changes between
  creation and settlement, or when the original remittance had a volume discount

Also:
- Fix pre-existing check_rate_limit ambiguity (abuse_protection vs rate_limit glob
  imports) by qualifying the three call sites with abuse_protection::
- Switch reqwest dev-dependency to rustls-tls to remove the pkg-config/openssl
  system dependency that blocked local 'cargo test' runs
- Fix test_dispute, test_contract_upgrade, test_features_589_592, test_invariants:
  add missing 'integrator' (Option<Address>) argument to create_remittance calls

Closes Haroldwonder#1269 (SR-126)
Closes Haroldwonder#1270 (SR-127)
Closes Haroldwonder#1271 (SR-128)
Closes Haroldwonder#1272 (SR-129)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment