Skip to content

Withdrawals rework in ENTITIES - #5997

Merged
lehins merged 12 commits into
masterfrom
td/entities-withdrawals-rework
Sep 10, 2026
Merged

Withdrawals rework in ENTITIES#5997
lehins merged 12 commits into
masterfrom
td/entities-withdrawals-rework

Conversation

@teodanciu

@teodanciu teodanciu commented Aug 10, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Description

According to the spec, ENTITIES has the following responsibilities with respect to withdrawals:

  1. Network check
  2. Threaded existence (every withdrawal cred exists in the accounts state as it stands)
  3. Legacy only: full drain (each top withdrawal must equal the account's threaded balance exactly)
  4. Legacy only: sub-aggregate cap (sub-tx withdrawals per account, aggregated across subs, must not exceed pre-batch balance.)
  5. Non-legacy only: pre-batch existence (every withdrawal cred exists in original accounts)
  6. Non-legacy only: aggregate cap (batch): top + all subs, per account, aggregated, capped against pre-batch balance.
  7. Apply withdrawals

This is what this PR is achieving, alongside some tests and some clean-up.

Checklist

  • Commits in meaningful sequence and with useful messages.
  • Tests added or updated when needed.
  • CHANGELOG.md files updated for packages with externally visible changes.
    NOTE: New section is never added with the code changes. (See RELEASING.md).
  • Versions updated in .cabal and CHANGELOG.md files when necessary, according to the
    versioning process.
  • Version bounds in .cabal files updated when necessary.
    NOTE: If bounds change in a cabal file, that package itself must have a version increase. (See RELEASING.md).
  • Code formatted (use scripts/fourmolize.sh).
  • Cabal files formatted (use scripts/cabal-format.sh).
  • CDDL files are up to date (use cleret cabal run generate-cddl)
  • hie.yaml updated (use scripts/gen-hie.sh).
  • Self-reviewed the diff.

Copilot AI lite review requested due to automatic review settings August 10, 2026 15:09
@teodanciu
teodanciu requested a review from a team as a code owner August 10, 2026 15:09
@teodanciu teodanciu changed the title Entities withdrawals rework Withdrawals rework in ENTITIES Aug 10, 2026
Copilot AI reviewed Aug 10, 2026
View reviewed changes

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.

Pull request overview

This PR reworks how withdrawals are validated and reported in the Dijkstra era by moving batch-wide withdrawal checks into the ENTITIES rule (instead of UTXO), adding clearer predicate failures for missing “original” accounts, and expanding Imp tests to cover batch aggregation and legacy-mode edge cases.

Changes:

  • Introduce withdrawalsWithUnacceptableAmount and a shared lookupAccountAddress helper in cardano-ledger-core, and update internal withdrawal categorization to use it.
  • Move/remove Dijkstra batch-withdrawal validation from UTXO and implement aggregated (top + subtx) withdrawal validation in ENTITIES, with new failures for missing original accounts.
  • Update Dijkstra/Conway Imp tests and failure expectations to match the new validation behavior.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
libs/cardano-ledger-core/src/Cardano/Ledger/State/Account.hs Adds a new withdrawal-filtering helper and centralizes account-address lookup (network-aware).
eras/dijkstra/impl/src/Cardano/Ledger/Dijkstra/Rules/Entities.hs Implements new aggregated withdrawal checks (legacy vs non-legacy) and adds missing-original-account failures.
eras/dijkstra/impl/src/Cardano/Ledger/Dijkstra/Rules/Ledger.hs Updates ENTITIES signal/environment wiring to pass the full annotated tx and original accounts.
eras/dijkstra/impl/src/Cardano/Ledger/Dijkstra/Rules/Utxo.hs Removes the prior batch-withdrawal validation and corresponding predicate failure/CBOR tags.
eras/dijkstra/impl/src/Cardano/Ledger/Dijkstra/Rules/SubEntities.hs Reuses shared withdrawal-missing validations and maps new ENTITIES failures into sub-rule failures.
eras/dijkstra/impl/src/Cardano/Ledger/Dijkstra/Rules/SubUtxo.hs Drops the now-removed WithdrawalsExceedAccountBalance “impossible” case.
eras/dijkstra/impl/testlib/Test/Cardano/Ledger/Dijkstra/Imp/EntitiesSpec.hs Updates/extends Imp tests for batch aggregation, legacy-mode behavior, and updated failure sets.
eras/conway/impl/testlib/Test/Cardano/Ledger/Conway/Imp/CertsSpec.hs Adds a Conway Imp test clarifying wrong-network withdrawal behavior and associated failures.
eras/dijkstra/impl/CHANGELOG.md Updates changelog entries to reflect removal of the old Dijkstra withdrawal failure.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread libs/cardano-ledger-core/src/Cardano/Ledger/State/Account.hs Outdated
@teodanciu
teodanciu force-pushed the td/entities-withdrawals-rework branch 4 times, most recently from 0f9d95e to 1db4272 Compare August 10, 2026 23:02
@teodanciu
teodanciu force-pushed the td/entities-withdrawals-rework branch from 1db4272 to a028427 Compare August 17, 2026 18:44
@github-actions

github-actions Bot commented Aug 17, 2026 •
edited
Loading

Copy link
Copy Markdown

Test Failures

@teodanciu
teodanciu force-pushed the td/entities-withdrawals-rework branch 2 times, most recently from 0255b9d to 70c9998 Compare August 22, 2026 20:20
@teodanciu
teodanciu changed the base branch from master to td/conservation-of-value-in-legacy-mode August 22, 2026 20:20
@teodanciu
teodanciu force-pushed the td/entities-withdrawals-rework branch 2 times, most recently from f37c6f0 to fdbc28d Compare August 23, 2026 14:04
@teodanciu
teodanciu force-pushed the td/entities-withdrawals-rework branch from fdbc28d to 9b68736 Compare August 24, 2026 12:15
@teodanciu
teodanciu force-pushed the td/entities-withdrawals-rework branch from 9b68736 to 0aafdd1 Compare August 24, 2026 17:14
@teodanciu
teodanciu force-pushed the td/entities-withdrawals-rework branch from 0aafdd1 to 89d3375 Compare August 24, 2026 19:50
@teodanciu
teodanciu force-pushed the td/entities-withdrawals-rework branch from 89d3375 to 9b45352 Compare August 24, 2026 23:23
@teodanciu
teodanciu force-pushed the td/entities-withdrawals-rework branch from 9b45352 to 39320fe Compare August 25, 2026 08:20
Base automatically changed from td/conservation-of-value-in-legacy-mode to master August 28, 2026 18:29
@teodanciu
teodanciu force-pushed the td/entities-withdrawals-rework branch from 39320fe to 7e8c9cd Compare August 30, 2026 20:29
@teodanciu
teodanciu force-pushed the td/entities-withdrawals-rework branch 7 times, most recently from 3ae3d30 to ee48dc1 Compare September 3, 2026 19:40
lehins
lehins reviewed Sep 8, 2026
View reviewed changes
Comment thread eras/dijkstra/impl/src/Cardano/Ledger/Dijkstra/Rules/Entities.hs Outdated
Comment thread eras/dijkstra/impl/src/Cardano/Ledger/Dijkstra/Rules/Entities.hs Outdated
Comment thread eras/dijkstra/impl/src/Cardano/Ledger/Dijkstra/Rules/Entities.hs Outdated
@teodanciu
teodanciu force-pushed the td/entities-withdrawals-rework branch 4 times, most recently from e21dae2 to 8413eda Compare September 9, 2026 21:57
@teodanciu

teodanciu commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

More tests to come in another PR that has now become stale, I will bring that up-to date next.

@teodanciu
teodanciu force-pushed the td/entities-withdrawals-rework branch from 8413eda to f1287e7 Compare September 9, 2026 22:22
@teodanciu
Clarify behavior of Conway withdrawals with a wrong network
b1c8b7d
@teodanciu
Rename constructors in (Sub)EntitiesPredFailure for clarity
fb0166a
@teodanciu
Add new predicate failure for withdrawal accounts missing pre-batch
08cffb4
@teodanciu
Make SUBENTITIES withdrawal-accounts existence checks network-aware
13c2b1a 
Fire `SubWithdrawalAccountsMissing`/
`SubWithdrawalAccountsMissingPreBatch` if the network is wrong, even if
the stake credential is registered.
@teodanciu
Make (SUB)ENTITIES DD-accounts existence checks network aware
20443d1
@teodanciu
Rewrite validateWithdrawals to cover all spec requirements
e53fd9d
@teodanciu
Remove withdrawal validation from UTXO rule
bc9b26e 
now replaced by logic in ENTITIES
@teodanciu
Add more tests for withdrawals and direct deposits
2b4e3f8
@teodanciu
Improve withdrawal-values generation in EntitiesSpec
aada22b
@teodanciu
Add tests checking withdrawals funded by direct deposits
a82ec3c
@teodanciu
Split validateWithdrawals into original- and current-account checks
a29ec30
@teodanciu
Short-circuit current-account withdrawal checks on prior failure
2502efa
@teodanciu
teodanciu force-pushed the td/entities-withdrawals-rework branch from f1287e7 to 2502efa Compare September 10, 2026 10:45
lehins
lehins approved these changes Sep 10, 2026
View reviewed changes

@lehins lehins left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Awesome work!!!!
Thank you!

@lehins
lehins merged commit 9d4b20f into master Sep 10, 2026
50 of 51 checks passed
@lehins
lehins deleted the td/entities-withdrawals-rework branch September 10, 2026 22:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort, defaults to Lite
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@lehins lehins lehins approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@teodanciu @lehins