Fix RBAC storage/recovery, liquidity router pool allowlist, batch payment ID collisions, dispute test coverage - #1040
Merged
martinzhames merged 2 commits intoSep 1, 2026
Conversation
…ment ID collisions, and dispute test coverage - Issue dupdab#1026: Move rbac_access role storage from instance() to persistent(); add SuperAdmin tracking, block self-revocation, and block removing the last SuperAdmin so the contract can't be permanently locked out. - Issue dupdab#1025: Add an admin-managed pool allowlist to liquidity_router (initialize/add_pool/remove_pool) and reject unapproved pool addresses in check_and_route before the cross-contract call. - Issue dupdab#1024: Derive batch_payments payment IDs from merchant + a monotonically increasing per-contract counter + payment contents instead of ledger sequence + batch index, so IDs can't collide across merchants or concurrent batches. Persist PaymentRecord on-chain for auditability. - Issue dupdab#1023: Document dispute()'s intended behavior after a partial release, and add tests covering dispute-after-partial-release resolved to both the customer and the merchant. Closes dupdab#1026 dupdab#1025 dupdab#1024 dupdab#1023
|
@MJigah 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! 🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes 4 issues assigned to me in the Stellar Program.
Changes
rbac_accessrole storage frominstance()topersistent()(instance storage is loaded on every single contract call and doesn't scale with role count). AddSuperAdmintracking, block self-revocation, and block removing the lastSuperAdminso the contract can't be permanently locked out.liquidity_router(initialize/add_pool/remove_pool) and reject unapprovedpool_addressvalues incheck_and_routebefore the cross-contract call, so an attacker-deployed contract can't fabricate reserve values.batch_paymentspayment IDs from the merchant address, a monotonically increasing per-contract counter, and the payment's own contents — instead of just ledger sequence + batch index — so IDs can't collide across merchants or concurrent batches landing in the same ledger. Also persistPaymentRecordon-chain for auditability (previously defined but never used).dispute()'s intended behavior when called after a partial release, and add tests covering dispute-after-partial-release resolved to both the customer and the merchant (previously untested).Also fixed
payment_escrow/src/test.rs'ssetup_env()had unresolved git merge-conflict markers onmainand called the contract constructor with the wrong number of arguments on both sides of the conflict — needed a workingsetup_env()to add the #1023 tests. Left every other pre-existing conflicted test in that file untouched, as that's unrelated to these 4 issues.Also fixed two latent comparison bugs (
Address == &Addresstype mismatches from misreadingVec::iter()'s item type) introduced while writing the #1025/#1026 fixes, caught before commit.Closes #1026
Closes #1025
Closes #1024
Closes #1023