Skip to content

Expose direct ADA deposits in Dijkstra transaction builders - #1469

Merged
Jimbo4350 merged 2 commits into
masterfrom
feature/cip159-account-build-flags
Oct 16, 2026
Merged

Jimbo4350 merged 2 commits into
masterfrom
feature/cip159-account-build-flags

Conversation

@Jimbo4350

@Jimbo4350 Jimbo4350 commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Context

Dijkstra transaction builders could not specify direct ADA deposits into registered accounts. This adds the repeatable --direct-deposit STAKE_ADDRESS+LOVELACE option to transaction build, build-raw, build-estimate, and sub-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

  • Built the executable and both test suites with GHC 9.12.4 and the pinned dependencies.
  • All 120 functional tests passed, including a regression test that duplicate direct-deposit accounts are rejected before node access with a nonexistent socket. New tests decode top-level and signed nested transactions to verify single and multiple recipients, exercise zero and Word64 boundary amounts, reject invalid amounts/addresses and duplicate recipients, check era restrictions, and verify that deposits to multiple recipients reduce change during offline fee balancing.
  • All 821 golden tests passed, including command help snapshots.
  • Fourmolu, HLint, cabal-gild, and git diff --check passed. 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-deposit integration test is deferred until the updated CLI version is propagated to cardano-testnet. This exposes existing API/ledger functionality and does not establish mainnet activation or change ledger rules.

Checklist

  • Commit sequence broadly makes sense and commits have useful messages
  • Deposit tests are added and help snapshots are updated
  • Self-reviewed the diff
  • Changelog fragment added in .changes/

Jimbo4350 added a commit that referenced this pull request Oct 15, 2026
@Jimbo4350 Jimbo4350 changed the title Expose CIP-159 ADA account fields in Dijkstra transaction builders Expose direct ADA deposits in Dijkstra transaction builders Oct 15, 2026
@Jimbo4350
Jimbo4350 force-pushed the feature/cip159-account-build-flags branch 3 times, most recently from d6a9ced to 42523c2 Compare October 15, 2026 17:25
@Jimbo4350
Jimbo4350 marked this pull request as ready for review October 15, 2026 17:36
Copilot AI balanced review requested due to automatic review settings October 15, 2026 17:36

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 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-deposit options 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.

@Jimbo4350
Jimbo4350 force-pushed the feature/cip159-account-build-flags branch 2 times, most recently from cd66085 to 7fc8240 Compare October 15, 2026 17:40
@Jimbo4350
Jimbo4350 force-pushed the feature/cip159-account-build-flags branch from 7fc8240 to cea9124 Compare October 15, 2026 19:12

@carbolymer carbolymer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Comment thread cardano-cli/src/Cardano/CLI/EraBased/Common/Option.hs Outdated
Comment thread cardano-cli/src/Cardano/CLI/EraBased/Transaction/Run.hs
-- | 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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread cardano-cli/test/cardano-cli-test/Test/Cli/Transaction/DirectDeposit.hs Outdated
Comment thread cardano-cli/src/Cardano/CLI/Type/Error/TxCmdError.hs Outdated
@Jimbo4350
Jimbo4350 force-pushed the feature/cip159-account-build-flags branch from 34e09e0 to 5990207 Compare October 16, 2026 14:55
@Jimbo4350
Jimbo4350 enabled auto-merge October 16, 2026 15:01
@Jimbo4350
Jimbo4350 added this pull request to the merge queue Oct 16, 2026
Merged via the queue into master with commit 6d33579 Oct 16, 2026
25 checks passed
@Jimbo4350
Jimbo4350 deleted the feature/cip159-account-build-flags branch October 16, 2026 16:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants