Skip to content

Conversation

@Soupstraw
Copy link
Contributor

@Soupstraw Soupstraw commented Nov 27, 2025

Description

This PR adds DijkstraUTXO together with a test that prevents using collateral return with a Ptr.

close #5454

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.

@Soupstraw Soupstraw force-pushed the jj/prevent-ptrs-collreturn branch from 24d0ccb to 669b2a0 Compare November 27, 2025 14:38
@Soupstraw Soupstraw changed the title Added Dijkstra UTXO rule and collateral Ptr check Added Dijkstra UTXO rule and collateral Ptr check Nov 27, 2025
@Soupstraw Soupstraw marked this pull request as ready for review November 27, 2025 15:06
@Soupstraw Soupstraw requested a review from a team as a code owner November 27, 2025 15:06
@Soupstraw Soupstraw force-pushed the jj/prevent-ptrs-collreturn branch from 487ceae to 170e595 Compare November 27, 2025 15:06
@Soupstraw Soupstraw force-pushed the jj/prevent-ptrs-collreturn branch from 170e595 to 6d10eec Compare November 28, 2025 10:10
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.

Looks good to me, except for this which I believe we don't want to do:

data DijkstraUtxoPredFailure era = ConwayInDijkstraUtxoPredFailure (ConwayUtxoPredFailure era)

, Signal (EraRule "UTXO" era) ~ Tx TopTx era
, BaseM (EraRule "UTXO" era) ~ ShelleyBase
, STS (EraRule "UTXO" era)
, -- In this function we we call the UTXOS rule, so we need some assumptions
Copy link
Contributor

Choose a reason for hiding this comment

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

Accidental double

Suggested change
, -- In this function we we call the UTXOS rule, so we need some assumptions
, -- In this function we call the UTXOS rule, so we need some assumptions

, Signal (EraRule "UTXO" era) ~ Tx TopTx era
, BaseM (EraRule "UTXO" era) ~ ShelleyBase
, STS (EraRule "UTXO" era)
, -- In this function we we call the UTXOS rule, so we need some assumptions
Copy link
Contributor

Choose a reason for hiding this comment

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

This comment no longer holds I believe

Suggested change
, -- In this function we we call the UTXOS rule, so we need some assumptions


{- ‖collateral tx‖ ≤ maxCollInputs pp -}
runTest $ Alonzo.validateTooManyCollateralInputs pp txBody
pure utxos
Copy link
Contributor

Choose a reason for hiding this comment

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

Might it be better to return unit perhaps, to make it clear that the function doesn't change the given state?

import Validation (failure)

data DijkstraUtxoPredFailure era
= ConwayInDijkstraUtxoPredFailure (ConwayUtxoPredFailure era)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this wrapping of the predicate failure from the previous era is problematic - ( I don't remember exactly why...perhaps something to do with deserialization).

This issue seems to be about removing these kind of things from the code, if I'm not mistaken: #5299

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.

Prevent Ptrs in collateral return

3 participants