Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mempool rule #4642

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Mempool rule #4642

wants to merge 6 commits into from

Conversation

teodanciu
Copy link
Contributor

@teodanciu teodanciu commented Sep 23, 2024

Description

Introduces Mempool rule and calls it from Ledger rule for transactions in the mempool.

The first commit adds the predicate failure to ConwayLedgerPredFailure, which breaks node-to-client protocol.
If the rest of the PR requires much rework, which would detract too much from Issues discovered in Conway and fixed during the bootstrap phase, then we can just merge the first commit and put the rest on hold for after the intra-era HF.

Closes #4622

Checklist

  • Commit sequence broadly makes sense and commits have useful messages
  • New tests are added if needed and existing tests are updated
  • All visible changes are prepended to the latest section of a CHANGELOG.md for the affected packages.
    New section is never added with the code changes. (See RELEASING.md)
  • When applicable, versions are updated in .cabal and CHANGELOG.md files according to the
    versioning process.
  • The version bounds in .cabal files for all affected packages are updated.
    If you change the bounds in a cabal file, that package itself must have a version increase. (See RELEASING.md)
  • Code is formatted with fourmolu (use scripts/fourmolize.sh)
  • Cabal files are formatted (use scripts/cabal-format.sh)
  • hie.yaml has been updated (use scripts/gen-hie.sh)
  • Self-reviewed the diff

@teodanciu teodanciu changed the title [WIP] Mempool rule Mempool rule Sep 25, 2024
@teodanciu teodanciu marked this pull request as ready for review September 25, 2024 20:38
@teodanciu teodanciu requested a review from a team as a code owner September 25, 2024 20:38
ls
(Seq.singleton tx)
let mempoolEvents = [ev | LedgerEvent ev@(MempoolEvent (ConwayMempoolEvent _)) <- evs]
mempoolEvents `shouldBeExpr` []
Copy link
Contributor Author

@teodanciu teodanciu Sep 26, 2024

Choose a reason for hiding this comment

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

EDIT: NEVERMIND!!! I realized it's not possible to get events out of applyTxs anyway! Everything I wrote below assumes the opposite, so nevermind! I will leave the comment in order to avoid confusion if an email was sent already.

I tried to write this test:

    it "Mempool events should be emitted via applyTxs" $ do
      let tx = mkBasicTx mkBasicTxBody
      globals <- use impGlobalsL
      slotNo <- use impLastTickG
      nes <- use impNESL

      _ <-  $ applyTxs globals slotNo (Seq.singleton tx) nes   -- and try to get events somehow

But in order to use applyTxs, it appears that I need a MonadError (ApplyTxError era) (ImpTestM era) instance .

I tried to write it, but failed and gave up!
Tried something like this:

  (Era era, Show (PredicateFailure (EraRule "LEDGER" era))) =>
     MonadError (ApplyTxError era) (ImpTestM era)
  where
  throwError (ApplyTxError predFailures) = fail (show predFailures)

  catchError (ImpTestM action) handler =  action  --  I don't know how to thread in the predFailures from the handler   

I don't know 1) whether all of this is worth it, or 2) whether it's all misguided and there is some straightforward way to call applyTxs within ImpTestM or 3) whether it should make sense at all to call applyTxs within ImptestM.

Copy link
Collaborator

@lehins lehins Sep 26, 2024

Choose a reason for hiding this comment

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

whether it's all misguided and there is some straightforward way to call applyTxs within ImpTestM or

What you need is use runExcept in order to get applyTx out of MonadError

Copy link
Collaborator

@lehins lehins Sep 26, 2024

Choose a reason for hiding this comment

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

Or even simpler just restrict it to Either:

case applyTxs globals slotNo (Seq.singleton tx) nes of
  Left err -> error $ show err
  Right (st, vTx) -> ...

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 MEMPOOL rule
2 participants