Skip to content

ConwayCERTS: Predicate failure for incomplete withdrawals#5279

Closed
aniketd wants to merge 1 commit intomasterfrom
aniketd/withdrawal-mismatch
Closed

ConwayCERTS: Predicate failure for incomplete withdrawals#5279
aniketd wants to merge 1 commit intomasterfrom
aniketd/withdrawal-mismatch

Conversation

@aniketd
Copy link
Contributor

@aniketd aniketd commented Sep 8, 2025

Description

Add IncompleteWithdrawalsCERTS to ConwayPredFailure conditional upon protocol version 11, via hardforkConwayCERTSIncompleteWithdrawals.

Closes #4640

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 scripts/gen-cddl.sh)
  • hie.yaml updated (use scripts/gen-hie.sh).
  • Self-reviewed the diff.

@aniketd aniketd changed the title ConwayCERT: Predicate failure for incomplete withdrawals ConwayCERTS: Predicate failure for incomplete withdrawals Sep 8, 2025
@aniketd aniketd force-pushed the aniketd/withdrawal-mismatch branch 3 times, most recently from e916734 to 8b75109 Compare September 9, 2025 12:50
Conditional upon protocol version 11.
Adjust the relevant tests.
@aniketd aniketd force-pushed the aniketd/withdrawal-mismatch branch from 8b75109 to b0f397a Compare September 9, 2025 12:58
@aniketd aniketd marked this pull request as ready for review September 9, 2025 13:00
@aniketd aniketd requested a review from a team as a code owner September 9, 2025 13:00
Copy link
Contributor

@teodanciu teodanciu left a comment

Choose a reason for hiding this comment

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

Some questions from me.
It looks good to me, but I don't entirely trust myself to approve the tuple business and the bangs usage in collectBadWithdrawals, so I'll leave to Alexey to approve.

Just accountState
| raNetwork /= networkId ->
first (Map.insert ra withdrawalAmount) accum
| fromCompact (accountState ^. balanceAccountStateL) /= withdrawalAmount ->
Copy link
Contributor

Choose a reason for hiding this comment

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

If it's both in the wrong network and the amount is incorrect, it's enough to return a predicate for the former?

accum@(!_, !_) =
case Map.lookup raCredential (accounts ^. accountsMapL) of
Nothing ->
first (Map.insert ra withdrawalAmount) accum
Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting, TIL about this.
Just curious, is there any advantage in doing this rather than binding the two components of the tuple and then using the first one here?

@@ -2,6 +2,7 @@

## 1.20.0.0
Copy link
Contributor

Choose a reason for hiding this comment

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

As the ledger release happened, we'll have to change the version in the cabal file to 1.20.1.0 (or maybe even 1.21.1.0?) , and then also the headline in the CHANGELOG.


## 1.20.0.0

* Add `IncompleteWithdrawalsCERTS` to `ConwayPredFailure`.
Copy link
Contributor

Choose a reason for hiding this comment

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

You could also mention hardforkConwayCERTSIncompleteWithdrawals

Suggested change
* Add `IncompleteWithdrawalsCERTS` to `ConwayPredFailure`.
* Add `IncompleteWithdrawalsCERTS` to `ConwayPredFailure`.
* Add `hardforkConwayCERTSIncompleteWithdrawals`

@aniketd
Copy link
Contributor Author

aniketd commented Sep 15, 2025

Closing in favour of #5291

@aniketd aniketd closed this Sep 15, 2025
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.

Add a new predicate failure for non-matching withdrawals

2 participants