fix(rs-platform-wallet/e2e): bank.fund_address pays fee from input [QA-001b]#3579
Merged
lklimek merged 3 commits intofeat/rs-platform-wallet-e2efrom May 5, 2026
Merged
Conversation
…ot recipient [QA-001b]
bank.fund_address previously used `[ReduceOutput(0)]`, so recipients
received `funding_per - fee` instead of the requested `funding_per`.
Tests calling `register_identity_from_addresses({addr: funding_per}, ...)`
then failed with `AddressesNotEnoughFundsError` because the address
balance was lower than the requested input amount.
Switch the bank to `[DeductFromInput(0)]` so the bank's own input
absorbs the fee and the recipient receives the exact requested
amount. Other call sites of `default_fee_strategy()` are
unaffected — they keep the `ReduceOutput(0)` semantics that PA tests
rely on.
Reported by Marvin (id_003 retest after QA-001 wait-threshold fix).
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
…nput(0) bank semantics Under Option C, bank.fund_address uses [DeductFromInput(0)] so the recipient receives the exact requested amount; the bank's fee is deducted from its own input and is invisible to the test wallet. The previous arithmetic derived bank_fee from (FUNDING_CREDITS - observed_total - transfer_fee), which under the new strategy always computes 0 — making the bank_fee > 0 assertion at transfer.rs:169 fail unconditionally. Replace with a direct bank.total_credits() pre/post comparison: capture bank_pre before fund_address, resync the bank afterward, capture bank_post, then bank_fee = bank_pre - bank_post - FUNDING_CREDITS. Also adds an assert_eq! that addr_1 retains FUNDING_CREDITS minus only the self-transfer fee (not the bank fee), and updates const comments to reflect DeductFromInput(0) semantics for the bank step. Reported by Marvin (PA-001 smoke test on PR #3579). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ReduceOutput(0)] semantics [QA-104] Previous fix updated bank_fee derivation (bc3fe60) but left the addr_1 retention assertion wrong: it asserted addr_1 == FUNDING_CREDITS - transfer_fee, but the protocol's [ReduceOutput(0)] strategy deducts the fee from output[0] (addr_2's amount), not from addr_1's residual. Correct math: - addr_1 retains FUNDING_CREDITS - TRANSFER_CREDITS (what was sent leaves). - addr_2 receives TRANSFER_CREDITS - transfer_fee (post-fee delivery). Reported by Marvin (suite-2 PA-001 rerun on PR #3579). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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
1 participant
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.
Issue being fixed or feature implemented
Stacks on top of #3549.
QA-001b discovered while running the identity e2e suite (
id_003_identity_to_identity_credit_transfer): `bank.fund_address(addr, N)` previously used `[ReduceOutput(0)]` fee strategy, so the recipient received `N - fee` (~`N - 9.5M` on testnet) instead of `N`. Tests calling `register_identity_from_addresses({addr: N}, ...)` then failed with `AddressesNotEnoughFundsError` because the address balance was lower than the requested input amount.This is a framework bug — `bank.fund_address` couples its funding contract with the protocol's fee model in a way that surprises every caller. Tests have to either compensate by passing post-fee amounts everywhere, or by pre-funding extra. Both leak the fee model into test logic that shouldn't care.
What was done?
Switched `bank.fund_address` to `[DeductFromInput(0)]` (already supported by the wallet's `auto_select_inputs` selector at `packages/rs-platform-wallet/src/wallet/platform_addresses/transfer.rs:246`). The bank's own input absorbs the fee; the recipient receives the exact requested amount.
How Has This Been Tested?
Breaking Changes
None. Test-framework only; no change to wallet API.
Checklist:
🤖 Co-authored by Claudius the Magnificent AI Agent