Repository navigation
Expose direct ADA deposits in Dijkstra transaction builders - #1469
Conversation
d6a9ced to
42523c2
Compare
There was a problem hiding this comment.
🔵 Needs a closer look
Human review should confirm node-backed balancing and ledger integration for this monetary feature beyond the reported offline tests.
0 open findings
What changed in this PR
Adds direct ADA deposits to Dijkstra transaction builders, using existing API support.
Changes:
- Adds repeatable, Dijkstra-only
--direct-depositoptions with duplicate-recipient validation. - Passes deposits through transaction construction, balancing, and signed sub-transactions.
- Adds functional tests, help snapshots, documentation, and a changelog fragment.
| File | Description |
|---|---|
| cardano-cli/test/cardano-cli-test/Test/Cli/Transaction/DirectDeposit.hs | Tests deposits, validation, era restrictions, and balancing. |
| cardano-cli/test/cardano-cli-golden/files/golden/help/dijkstra_transaction_build.cli | Updates build help. |
| cardano-cli/test/cardano-cli-golden/files/golden/help/dijkstra_transaction_build-raw.cli | Updates raw-build help. |
| cardano-cli/test/cardano-cli-golden/files/golden/help/dijkstra_transaction_build-estimate.cli | Updates estimate help. |
| cardano-cli/test/cardano-cli-golden/files/golden/help/dijkstra_sub-transaction_build-raw.cli | Updates sub-transaction help. |
| cardano-cli/test/cardano-cli-golden/files/golden/help.cli | Updates combined help snapshot. |
| cardano-cli/src/Cardano/CLI/Type/Error/TxCmdError.hs | Adds duplicate-deposit errors. |
| cardano-cli/src/Cardano/CLI/EraBased/Transaction/SubTransaction/Run.hs | Includes deposits in sub-transactions. |
| cardano-cli/src/Cardano/CLI/EraBased/Transaction/SubTransaction/Option.hs | Connects sub-transaction deposit parsing. |
| cardano-cli/src/Cardano/CLI/EraBased/Transaction/SubTransaction/Command.hs | Adds deposit arguments. |
| cardano-cli/src/Cardano/CLI/EraBased/Transaction/Run.hs | Passes deposits into transaction builders. |
| cardano-cli/src/Cardano/CLI/EraBased/Transaction/Option.hs | Connects deposit parsing across builders. |
| cardano-cli/src/Cardano/CLI/EraBased/Transaction/Internal/Common.hs | Validates and converts deposits. |
| cardano-cli/src/Cardano/CLI/EraBased/Transaction/Command.hs | Adds builder deposit fields. |
| cardano-cli/src/Cardano/CLI/EraBased/Common/Option.hs | Defines Dijkstra-only deposit parsing. |
| cardano-cli/README.md | Documents deposit usage and scope. |
| cardano-cli/cardano-cli.cabal | Registers the test module. |
| .changes/20261015_cip159_account_build_flags.yml | Records the feature. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
cd66085 to
7fc8240
Compare
7fc8240 to
cea9124
Compare
| -- | With 100 ADA of inputs and direct deposits of 20 and 30 ADA, build-estimate | ||
| -- must return 50 ADA minus the estimated fee as change. This catches deposits being | ||
| -- recorded in the body but omitted from the change calculation. | ||
| hprop_cip159_build_estimate_deducts_deposits_from_change :: Property |
There was a problem hiding this comment.
This covers balancing with deposits through build-estimate.
build balances through makeTransactionBodyAutoBalance, and no test here runs that path with deposits.
Is a cardano-testnet test for build with --direct-deposit planned?
There was a problem hiding this comment.
Yes, this needs a follow-up after the updated CLI version is propagated to cardano-testnet; that dependency update is required before we can exercise transaction build --direct-deposit against a running node there. The build path is already wired up in this PR. The current balancing test covers build-estimate, not node-backed auto-balancing. The follow-up should cover building, signing and submitting a deposit transaction, then checking the recipient balance and change.
34e09e0 to
5990207
Compare
Context
Dijkstra transaction builders could not specify direct ADA deposits into registered accounts. This adds the repeatable
--direct-deposit STAKE_ADDRESS+LOVELACEoption totransaction build,build-raw,build-estimate, andsub-transaction build-raw.For example,
--direct-deposit "${ACCOUNT}+2000000"credits the account with 2 ADA, funded by the transaction. Deposits are retained in signed nested transactions and included in automatic fee balancing. Duplicate recipients, malformed amounts, and use in Conway/latest are rejected.This PR covers direct ADA deposits only. Balance assertions and starting-balance conditions are deferred to a follow-up PR. Existing withdrawal behavior is unchanged, and native-token deposits are out of scope.
Command help snapshots and a changelog fragment are included. The pinned cardano-api already supports direct deposits, so no dependency update is needed.
How to trust this PR
git diff --checkpassed. The local Git hook launcher references a missing Nix interpreter, so checks were run directly before committing.No live Dijkstra node submission or ledger acceptance test was run. A node-backed
transaction build --direct-depositintegration test is deferred until the updated CLI version is propagated tocardano-testnet. This exposes existing API/ledger functionality and does not establish mainnet activation or change ledger rules.Checklist
.changes/