cardano-ledger icon indicating copy to clipboard operation
cardano-ledger copied to clipboard

Implement block-submitting Imp functions

Open neilmayhew opened this issue 1 month ago • 5 comments

Description

Add functions to:

  • submit a list of fixed-up transactions in a block
  • capture submitted transactions in a "simulation" and then group them into a block and submit it

Checklist

  • [x] Commits in meaningful sequence and with useful messages.
  • [x] Tests added or updated when needed.
  • [x] CHANGELOG.md files updated for packages with externally visible changes.
    NOTE: New section is never added with the code changes. (See RELEASING.md).
  • [x] Versions updated in .cabal and CHANGELOG.md files when necessary, according to the versioning process.
  • [x] 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).
  • [x] Code formatted (use scripts/fourmolize.sh).
  • [x] Cabal files formatted (use scripts/cabal-format.sh).
  • [x] CDDL files are up to date (use scripts/gen-cddl.sh)
  • [x] hie.yaml updated (use scripts/gen-hie.sh).
  • [x] Self-reviewed the diff.

neilmayhew avatar Nov 10 '25 15:11 neilmayhew

OK, this is ready for another review now

neilmayhew avatar Dec 02 '25 01:12 neilmayhew

With the new commit to switch to using applyBlockEither, I think this is complete. As we discussed, I won't change trySubmitTx for now, but will make a follow-up PR after #5461 is merged.

neilmayhew avatar Dec 05 '25 00:12 neilmayhew

I had to change the design slightly, because there wasn't a way to submit a block containing failing transactions, because failing transactions aren't recorded. I therefore changed tryTxsInBlock to take an explicit list of (fixed-up) transactions and added a new wrapper function withTxsInBlockEither that has the previous functionality.

Unfortunately, tryTxsInBlock now has to take an additional finalState parameter because there's no longer a way to know what the final ImpTestState should be. This is ugly and I'd like to remove it, but it will require refactoring trySubmitTx to split out the functionality that updates the state, and this PR has hung around long enough already (and has required a couple of tricky rebases).

neilmayhew avatar Dec 06 '25 00:12 neilmayhew

I suppose another possibility would be to have trySubmitTx record all transactions, not just successful ones.

neilmayhew avatar Dec 06 '25 00:12 neilmayhew

I suppose another possibility would be to have trySubmitTx record all transactions, not just successful ones.

As we discussed, I won't do this. However, one of the tests in #5392 requires calling tryTxsInBlock directly with an explicitly fixed-up failing transaction in order to produce a failing block. Maybe there's a better way to do that, but we can discuss it in the other PR.

neilmayhew avatar Dec 08 '25 23:12 neilmayhew