fix(vault): align simulate_deduct with deduct via shared validate_deduct - #1337
Open
Manta-Byte wants to merge 1 commit into
Open
Manta-Byte wants to merge 1 commit into
Manta-Byte wants to merge 1 commit into
Conversation
…alloraOrg#1115) Extract validate_deduct used by both paths (authorized caller, pause, min/max bounds, balance). Drop slippage/rate-limit/duplicate checks from the simulator, align request_id to u64, update module docs, add 256-case parity proptest.
|
@Manta-Byte 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.
Closes #1115
Summary
simulate_deductclaimed parity withdeductbut checked different things. Both now call one sharedvalidate_deduct, so the preflight returns the same error the live call would for caller, pause state, amount bounds and balance.Criteria → code and tests
validate_deductinlib.rs, called bydeductand bysimulate_deductinviews.rsBelowMinDepositrequire_valid_deduct_amountinsidevalidate_deducttest_simulate_parity.rs: random amount, caller, paused flag and balance;ProptestConfig::with_cases(256); asserts identical error codes or both succeedviews.rsmodule docs rewrittenBehavior changes
simulate_deductsignature is now(env, caller, amount, request_id: u64). It previously tookOption<Symbol>plusmax_fee_bpsanddeveloper.deductnever implemented it, so the simulator produced false failures. Implementing it indeductwould change on-chain behavior, so I removed it from the simulator instead. Happy to go the other way if you prefer.deductdoesn't either.docs/interfaces/vault.jsonupdated to the newsimulate_deductsignature.Design and failure modes
validate_deductis read-only: no storage writes, no events, no auth. Check order and error values fordeductare unchanged (authorized caller, pause, amount bounds, balance).validate_deducttakes(env, caller, amount).request_idis not validated bydeduct, so it isn't passed in.simulate_deductkeeps the parameter for interface alignment.caller.require_auth()stays indeductonly, so the simulator can be called without auth and never panics on it.deductperforms around token transfer (settlement / USDC configuration), if any, are outside it.Compatibility
simulate_deductis a view, so there are no storage or state changes and existing deployments are unaffected. Callers of the old signature must update; I found none in this repo outside the vault crate (older status reports in the repo root describe a previous signature and were left untouched).proptestis added as a dev-dependency only, using the same feature set ascallora-settlement.Validation
cargo test -p callora-vault simulate