refactor: extract shared deposit validation from manual flow#174
Open
refactor: extract shared deposit validation from manual flow#174
Conversation
This was referenced Apr 24, 2026
Extracts the core "fetch tx → validate → accept" logic from `try_accept_deposit` into a new `fetch_and_validate_deposit` helper in `deposit/mod.rs`, so the upcoming automatic deposit flow can reuse it with a different fee. - `get_transaction` no longer takes a `cycles_to_attach` parameter; cycles are fixed via a new `GET_TRANSACTION_CYCLES` constant - `fetch_and_validate_deposit` takes a `fee: Lamport` parameter so both manual (`manual_deposit_fee`) and automatic (`automated_deposit_fee`) flows can reuse it - Cycle charging in `try_accept_deposit` now only adds `deposit_consolidation_fee` on success, not on every call No behavior change for valid deposits. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
lpahlavi
force-pushed
the
lpahlavi/defi-2780-extract-deposit-validation
branch
from
9b3c56f to
6dd2b9b
Compare
lpahlavi
added a commit
that referenced
this pull request
Apr 24, 2026
Squashed from lpahlavi/defi-2780-extract-deposit-validation: - Add `fetch_and_validate_deposit` shared helper to `deposit/mod.rs` - Extract `GET_TRANSACTION_CYCLES` constant (50 B cycles) - Remove `cycles_to_attach` parameter from `get_transaction` — it now uses the constant internally - Refactor `try_accept_deposit` to delegate to `fetch_and_validate_deposit` - Fix `Burn(#[n(0)])` → `Burn(#[n(1)])` in `Memo` cbor annotation Note: the `meta.err` / `TransactionFailed` removal from #174 was NOT cherry-picked; that is a bug in #174 that should be fixed separately. 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
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.
Summary
Extracts shared deposit validation logic from the manual deposit flow in preparation for the automatic deposit flow.
Changes:
rpc::get_transaction: removes thecycles_to_attachparameter — cycles are now fixed viaGET_TRANSACTION_CYCLESconstant inconstants.rs, since the automatic flow also needs to call it with the same amountdeposit::fetch_and_validate_deposit: shared async helper encapsulating the full validation pipeline — fetch transaction via RPC, validate it targets the deposit address, check minimum amount, computeamount_to_mint. Takes afee: Lamportparameter so both manual (manual_deposit_fee) and automatic (automated_deposit_fee) flows can reuse ittry_accept_deposit: simplified to callfetch_and_validate_deposit; cycle charging now only addsdeposit_consolidation_feeon success (not on every call)runtime_with_time_and_cyclesinto two helpers — one for valid-deposit tests (charges RPC cost + consolidation fee) and one for failure tests (charges RPC cost only)No behavior change for valid deposits.
🤖 Generated with Claude Code