Misha/issue 100 time conversions #105

mikekeke · 2022-05-24T10:39:47Z

Related issue: #100
This PR aims to fix time <-> slot conversions.

Changes:

new effects added for time -> slot, slot -> time and time range -> slot range conversions
plutus-ledger functions for time <-> slot conversions replaced with new ones, which re-use functions from ledger and should give correct conversions sustainable to slot length changes
pcSlotConfig removed from PABConfig as it is not required anymore

New conversions were tested on public testnet with this validator.

- v1 working variant, checked on testnet

- cleanup TimeSlot module - refactoring - removing `SlotConfig` from `PABConfig`

- testnet debugging enhancements

- minor refactoring and docs

- removing debug contract - formatting

src/BotPlutusInterface/Balance.hs

src/BotPlutusInterface/ExBudget.hs

src/BotPlutusInterface/TimeSlot.hs

samuelWilliams99 · 2022-05-31T10:50:37Z

src/BotPlutusInterface/TimeSlot.hs

+    let lowerB = LowerBound startSlot startSlotClosure
+        upperB = UpperBound endSlot endSlotClosure
+    pure $ Interval lowerB upperB
+    where


These are a litttle involved, lets get some type signatures on them

type signatures added

I think @samuelWilliams99 ment the convertExtended and getExtClosure functions, no?

Added signatures there as well and clarification comments.

src/BotPlutusInterface/TimeSlot.hs

samuelWilliams99 · 2022-05-31T10:52:41Z

test/Spec/MockContract.hs

    , teRawCBOR = fromRight (error "failed to unpack CBOR hex") $ unhex "84a500848258205d677265fa5bb21ce6d8c7502aca70b9316d10e958611f3c6b758f65ad9599960182582076ed2fcda860de2cbacd0f3a169058fa91eff47bc1e1e5b6d84497159fbc9300008258209405c89393ba84b14bf8d3e7ed4788cc6e2257831943b58338bee8d37a3668fc00825820a1be9565ccac4a04d2b5bf0d0167196ae467da0d88161c9c827fbe76452b24ef000d8182582076ed2fcda860de2cbacd0f3a169058fa91eff47bc1e1e5b6d84497159fbc930000018482581d600f45aaf1b2959db6e5ff94dbb1f823bf257680c3c723ac2d49f975461a3b8cc4a582581d600f45aaf1b2959db6e5ff94dbb1f823bf257680c3c723ac2d49f97546821a00150bd0a1581c1d6445ddeda578117f393848e685128f1e78ad0c4e48129c5964dc2ea14974657374546f6b656e1a000d062782581d606696936bb8ae24859d0c2e4d05584106601f58a5e9466282c8561b88821a00150bd0a1581c1d6445ddeda578117f393848e685128f1e78ad0c4e48129c5964dc2ea14974657374546f6b656e1282581d60981fc565bcf0c95c0cfa6ee6693875b60d529d87ed7082e9bf03c6a4821a00150bd0a1581c1d6445ddeda578117f393848e685128f1e78ad0c4e48129c5964dc2ea14974657374546f6b656e0f021a000320250e81581c0f45aaf1b2959db6e5ff94dbb1f823bf257680c3c723ac2d49f97546a10081825820096092b8515d75c2a2f75d6aa7c5191996755840e81deaa403dba5b690f091b65840295a93849a67cecabb8286e561c407b6bd49abf8d2da8bfb821105eae4d28ef0ef1b9ee5e8abb8fd334059f3dfc78c0a65e74057a2dc8d1d12e46842abea600ff5f6"
    }
+
+mockSlotRange ::


Given we're mocking this, can we get a test for it?

I think it will require this to make it clean and straightforward. There is bunch of node queries required to get all arguments for conversion and they need to be mocked also if we are not calling real node.
(there will be still some parts tho, that I'm not sure how get easily atm, like EraHistory)

test/Spec/MockContract.hs

szg251 · 2022-06-01T11:47:19Z

src/BotPlutusInterface/Effects.hs

  QueryChainIndex :: ChainIndexQuery -> PABEffect w ChainIndexResponse
  EstimateBudget :: TxFile -> PABEffect w (Either BudgetEstimationError TxBudget)
  SaveBudget :: Ledger.TxId -> TxBudget -> PABEffect w ()
+  SlotToPOSIXTime ::


I started to get a feeling that instead of introducing new effects here for each cardano-api related query, we should have a more general QueryNode that uses this (this would also make later attempts to use cardano-api instead of the cli or chain-index a bit simpler)

This is great idea, I think. It will also make things more testable (e.g. like this one).
But not sure if it should be done under this PR. To make it right budget execution related logic need to be changed also.

Or maybe just change part related to slot/time conversions to QueryNode effect and leave ex-budget for separate issue?

Agreed yes, let's make that a new issue (the problem with refactoring issues is that they tend to be forgotten, staying in the backlog forever, so I like to couple them with some useful feature)

Made an issue #109

szg251

There are still a few small ones

src/BotPlutusInterface/TimeSlot.hs

szg251 · 2022-06-02T15:37:23Z

src/BotPlutusInterface/TimeSlot.hs

+    let lowerB = LowerBound startSlot startSlotClosure
+        upperB = UpperBound endSlot endSlotClosure
+    pure $ Interval lowerB upperB
+    where


I think @samuelWilliams99 ment the convertExtended and getExtClosure functions, no?

- review comments resolved - link to sources for mintining policies sorting comment corrected - overlooked redundant Slot conversion in `convertExtended` removed

mikekeke added 5 commits May 17, 2022 17:41

wip: time <-> slot conversions

e50c779

wip: time slot <-> time conversions

aa382a4

- v1 working variant, checked on testnet

wip: slot <-> time convcersion

b706d77

- cleanup TimeSlot module - refactoring - removing `SlotConfig` from `PABConfig`

Merge branch 'master' into misha/issue-100-time-conversions

d4b2e3a

wip: slot <-> time convcersion

b56363e

- testnet debugging enhancements

mikekeke mentioned this pull request May 27, 2022

Incorrect slot <-> time conversions mlabs-haskell/plutip#70

Closed

2 tasks

wip: slot <-> time convcersion

d184cdb

- minor refactoring and docs

mikekeke force-pushed the misha/issue-100-time-conversions branch from 4ff096c to d184cdb Compare May 30, 2022 13:06

mikekeke added 2 commits May 31, 2022 10:41

slot <-> time convcersion cleanup

5d77660

- removing debug contract - formatting

cabal formatting

8454d26

mikekeke requested review from samuelWilliams99 and szg251 May 31, 2022 09:16

mikekeke marked this pull request as ready for review May 31, 2022 09:16

Merge branch 'master' into misha/issue-100-time-conversions

eda445f

samuelWilliams99 reviewed May 31, 2022

View reviewed changes

szg251 reviewed Jun 1, 2022

View reviewed changes

mikekeke added 2 commits June 1, 2022 15:53

resolving review comments

7534388

formatting

944b904

mikekeke requested review from samuelWilliams99 and szg251 June 2, 2022 07:34

szg251 reviewed Jun 2, 2022

View reviewed changes

mikekeke added 3 commits June 3, 2022 11:12

fixes

2f66ea7

- review comments resolved - link to sources for mintining policies sorting comment corrected - overlooked redundant Slot conversion in `convertExtended` removed

Merge branch 'master' into misha/issue-100-time-conversions

38fab81

formatting fix

7cff2e9

mikekeke requested a review from szg251 June 3, 2022 08:27

mikekeke mentioned this pull request Jun 3, 2022

Introduce QueryNode effect for interactions with node #109

Closed

szg251 approved these changes Jun 3, 2022

View reviewed changes

samuelWilliams99 approved these changes Jun 3, 2022

View reviewed changes

mikekeke merged commit 0908ade into master Jun 3, 2022

mikekeke deleted the misha/issue-100-time-conversions branch June 3, 2022 12:34

This was referenced Jun 3, 2022

Query local node to convert between POSIXTime and SlotNo #100

Closed

Using a config file #91

Merged

Misha/issue 100 time conversions #105

Misha/issue 100 time conversions #105

Uh oh!

Conversation

mikekeke commented May 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

szg251 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mikekeke commented May 24, 2022 •

edited

Loading