Skip to content

Harden contract authorization, address invariants, payment ID derivation, and slippage validation - #187

Merged
terngunan merged 5 commits into
terngunan:mainfrom
sundayjob996:drips/93-94-95-96
Oct 21, 2026
Merged

terngunan merged 5 commits into
terngunan:mainfrom
sundayjob996:drips/93-94-95-96

Conversation

@sundayjob996

Copy link
Copy Markdown
Contributor

Summary

Harden contract authorization, address invariants, payment ID derivation, and slippage validation

What was solved

#93 — rbac_access: Hierarchical rank ordering permits OperationsAdmin to execute ComplianceAdmin restricted tasks

Fix rbac_access role authorization so ComplianceAdmin and OperationsAdmin are distinct functional roles instead of a linear rank hierarchy. Replace rank-based comparison with explicit role matching (or permission sets) so compliance tasks require ComplianceAdmin or SuperAdmin, and operations tasks require OperationsAdmin or SuperAdmin. Add unit tests verifying cross-role rejections and SuperAdmin access to both task types.

Addressed:

  • Changed: settliq_contracts/contracts/rbac_access/src/lib.rs, settliq_contracts/contracts/rbac_access/src/test.rs
  • Refactor role checking in settliq_contracts/contracts/rbac_access/src/lib.rs away from linear rank comparison to explicit role matching or permission sets
  • Ensure require_role(caller, Role::ComplianceAdmin) succeeds only for ComplianceAdmin or SuperAdmin, not OperationsAdmin
  • Ensure operations tasks require OperationsAdmin or SuperAdmin, not ComplianceAdmin

#94 — fee_distributor: set_treasury and set_lp_address setters omit address distinction and contract self-address invariants

Add constructor-equivalent address invariants to fee_distributor's set_treasury and set_lp_address setters so they reject setting treasury/lp_address to each other, to the admin, or to the contract's own address, and add unit tests covering the invalid permutations plus valid updates.

Addressed:

  • Changed: settliq_contracts/contracts/fee_distributor/src/lib.rs, settliq_contracts/contracts/fee_distributor/src/lib.rs, settliq_contracts/contracts/fee_distributor/src/test.rs
  • In set_treasury, assert treasury != lp_address, treasury != admin, and treasury != env.current_contract_address() before persisting/emitting the update event.
  • In set_lp_address, apply symmetric checks: lp_address != treasury, lp_address != admin, and lp_address != env.current_contract_address().
  • Preserve existing caller.require_auth() and Self::require_admin(&env, &caller) checks and the existing update event emission for valid updates.

#95 — batch_payments: Payment ID seed hash omits payment amount customer and memo despite documentation claims

Update batch_payments create_batch so the payment ID SHA-256 seed commits to the payment payload (amount, memo XDR, customer XDR) in addition to ledger sequence, counter, and merchant, matching the PaymentRecord documentation. Add a unit test in test.rs verifying that identical counter items with differing amounts/memos produce distinct payment IDs.

Addressed:

  • Changed: settliq_contracts/contracts/batch_payments/src/lib.rs
  • In settliq_contracts/contracts/batch_payments/src/lib.rs create_batch, append item.amount.to_be_bytes(), item.memo.to_xdr(&env), and item.customer.to_xdr(&env) to seed_bytes before sha256, keeping existing ledger sequence, counter, and merchant seed components.
  • Ensure the payment ID hash commits to amount, memo, and customer address.
  • Add a unit test in settliq_contracts/contracts/batch_payments/src/test.rs showing distinct payment IDs when amounts/memos vary for otherwise identical counter items.

#96 — slippage_protection: set_pair_max_slippage does not validate maximum basis points ceiling

Add the missing upper-bound validation to set_pair_max_slippage in the slippage_protection contract so per-pair overrides cannot exceed 10,000 bps, mirroring set_max_slippage, and add a unit test asserting rejection of bps = 10_001.

Addressed:

  • Changed: settliq_contracts/contracts/slippage_protection/src/lib.rs
  • In settliq_contracts/contracts/slippage_protection/src/lib.rs, add if bps > 10_000 { panic!("bps must be <= 10000"); } inside set_pair_max_slippage before storing the value, matching the existing check in set_max_slippage.
  • Ensure valid bps <= 10_000 continues to succeed and is stored under DataKey::PairMaxSlippageBps(asset_a, asset_b).
  • Add a unit test in settliq_contracts/contracts/slippage_protection/src/test.rs that asserts set_pair_max_slippage with bps = 10_001 panics (e.g. via #[should_panic]).

Changes

  • settliq_contracts/contracts/rbac_access/src/lib.rs (modify)
  • settliq_contracts/contracts/fee_distributor/src/lib.rs (modify)
  • settliq_contracts/contracts/batch_payments/src/lib.rs (modify)
  • settliq_contracts/contracts/rbac_access/src/test.rs (modify)
  • settliq_contracts/contracts/fee_distributor/src/test.rs (modify)
  • settliq_contracts/contracts/slippage_protection/src/lib.rs (modify)

Approach

  1. rbac_access: Hierarchical rank ordering permits OperationsAdmin to execute ComplianceAdmin restricted tasks #93 — rbac_access: Hierarchical rank ordering permits OperationsAdmin to execute ComplianceAdmin restricted tasks (Changed: settliq_contracts/contracts/rbac_access/src/lib.rs, settliq_contracts/contracts/rbac_access/src/test.rs)
  2. fee_distributor: set_treasury and set_lp_address setters omit address distinction and contract self-address invariants #94 — fee_distributor: set_treasury and set_lp_address setters omit address distinction and contract self-address invariants (Changed: settliq_contracts/contracts/fee_distributor/src/lib.rs, settliq_contracts/contracts/fee_distributor/src/lib.rs, settliq_contracts/contracts/fee_distributor/src/test.rs)
  3. batch_payments: Payment ID seed hash omits payment amount customer and memo despite documentation claims #95 — batch_payments: Payment ID seed hash omits payment amount customer and memo despite documentation claims (Changed: settliq_contracts/contracts/batch_payments/src/lib.rs)
  4. slippage_protection: set_pair_max_slippage does not validate maximum basis points ceiling #96 — slippage_protection: set_pair_max_slippage does not validate maximum basis points ceiling (Changed: settliq_contracts/contracts/slippage_protection/src/lib.rs)

Issues

Closes #93
Closes #94
Closes #95
Closes #96

@drips-wave

drips-wave Bot commented Oct 21, 2026

Copy link
Copy Markdown

@sundayjob996 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

@terngunan
terngunan merged commit 4bfd49e into terngunan:main Oct 21, 2026
0 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment