Skip to content
This repository was archived by the owner on Dec 2, 2024. It is now read-only.

PAB: More consistent slot configs#77

Merged
sjoerdvisscher merged 2 commits intoIntersectMBO:mainfrom
MELD-labs:pab-consistent-slot-config
Nov 8, 2021
Merged

PAB: More consistent slot configs#77
sjoerdvisscher merged 2 commits intoIntersectMBO:mainfrom
MELD-labs:pab-consistent-slot-config

Conversation

@kk-hainq
Copy link
Contributor

@kk-hainq kk-hainq commented Nov 1, 2021

This PR attempts to fix a misconfigured slot config issue we found while working with PAB. It started with a wrong Shelley launch time in plutus-pab/plutus-pab.yaml.sample, which leads to weird off-chain validation errors when we use mustValidateIn constraints.

The quick fix was to update the config file with the "correct" Shelley launch date, but this PR provides the general fix that removes the bug for good.

The flow starts with mustValidateIn, which adds the validity range to the UnbalancedTx:
https://github.com/input-output-hk/plutus-apps/blob/942bd8c6de6a2d5981d91c704b0258bddd9d9d7c/plutus-ledger/src/Ledger/Constraints/OffChain.hs#L505-L506

Which is eventually balanced here:
https://github.com/input-output-hk/plutus-apps/blob/942bd8c6de6a2d5981d91c704b0258bddd9d9d7c/plutus-contract/src/Wallet/Emulator/Wallet.hs#L202-L208

We can see that the original validity interval is translated to a slot range using the PAB's slot config before being set to UnbalancedTx's internal tx's validity range.

Fast forward through validateTxAndAddFees, then validateTransactionOffChain, then checkMintingScripts, then mkTxInfo we end up re-converting the slot range, but with the default slot config!
https://github.com/input-output-hk/plutus-apps/blob/ecc528ec38f42dee056ccb072abd41a64b640425/plutus-ledger/src/Ledger/Index.hs#L396

When the PAB's slot config does not match the default, this mismatch modifies the intended validity interval, and off-chain validates the transaction with it, which effectively makes the config not configurable.
https://github.com/input-output-hk/plutus-apps/blob/f4893f8713d299020d9e6d059dd956f30c11dca1/plutus-ledger/src/Ledger/TimeSlot.hs#L54-L56

Luckily the addition of SlotConfig to the existing API is pretty natural and matches previous works.

Pre-submit checklist:

  • Branch
    • Tests are provided (if possible)
    • Commit sequence broadly makes sense
    • Key commits have useful messages
    • Relevant tickets are mentioned in commit messages
    • Formatting, materialized Nix files, PNG optimization, etc. are updated
  • PR
    • Self-reviewed the diff
    • Useful pull request description
    • Reviewer requested

@kk-hainq kk-hainq changed the title PAB: More consistent slot config PAB: More consistent slot configs Nov 1, 2021
@kk-hainq kk-hainq force-pushed the pab-consistent-slot-config branch from 0ff7559 to a0da9a2 Compare November 3, 2021 19:27
@silky silky requested a review from sjoerdvisscher November 4, 2021 07:51
@kk-hainq kk-hainq force-pushed the pab-consistent-slot-config branch 2 times, most recently from a7a2068 to 7e9e0bd Compare November 4, 2021 23:42
@kk-hainq kk-hainq force-pushed the pab-consistent-slot-config branch from 7e9e0bd to ab091b8 Compare November 5, 2021 20:02
@kk-hainq kk-hainq force-pushed the pab-consistent-slot-config branch from ab091b8 to d44bc27 Compare November 6, 2021 07:44
@sjoerdvisscher
Copy link
Contributor

@kk-hainq This fix looks good! Is it ready?

@kk-hainq
Copy link
Contributor Author

kk-hainq commented Nov 8, 2021

@sjoerdvisscher It is, thanks! The recent force pushes were only rebase(s) to make sure it wouldn't break the recent major changes.

@sjoerdvisscher sjoerdvisscher merged commit 450ffef into IntersectMBO:main Nov 8, 2021
@sjoerdvisscher
Copy link
Contributor

Thanks @kk-hainq!

@kk-hainq kk-hainq deleted the pab-consistent-slot-config branch November 8, 2021 10:42
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants