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.

Sign up for GitHub

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

Jump to bottom

Validity and timelocks #3249

Merged
merged 31 commits into from
Apr 29, 2022
Merged

Validity and timelocks #3249

merged 31 commits into from
Apr 29, 2022

Conversation

paweljakubas
Copy link
Contributor

@paweljakubas paweljakubas commented Apr 22, 2022 •
edited
Loading

  • reading properly validity from request body
  • add checks on boundary precedence and quantity
  • simplify how asset quantity is called in constructTransaction (in line with decodeTransaction)
  • slot check
  • make either validity bound optional
  • introduce before bound to transaction context and propagate this to node's tx
  • extend decodeTransaction to detect txValidityInterval (with integration testing)
  • introducing slot computation of scripts
  • script vs validity check
  • timelock minting test on
  • add unit tests in MintBurnSpec

Comments

Issue Number

adp-1193

@paweljakubas paweljakubas self-assigned this Apr 22, 2022
@paweljakubas paweljakubas force-pushed the paweljakubas/adp-1193/validity-timelocks branch 4 times, most recently from e0217f4 to cff3a8f Compare April 28, 2022 13:22
@paweljakubas paweljakubas requested review from jonathanknowles and piotr-iohk and removed request for jonathanknowles April 28, 2022 19:46
@paweljakubas paweljakubas marked this pull request as ready for review April 28, 2022 19:46
@jonathanknowles jonathanknowles force-pushed the paweljakubas/adp-1193/validity-timelocks branch from 049430a to d9619f1 Compare April 29, 2022 01:33
@jonathanknowles
Copy link
Member

bors try

iohk-bors bot added a commit that referenced this pull request Apr 29, 2022
@iohk-bors
Try #3249:
41efc89
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Apr 29, 2022 •
edited by jonathanknowles
Loading

try

Build failed:

Looks like an hlint failure. Will fix and retry.

@jonathanknowles jonathanknowles force-pushed the paweljakubas/adp-1193/validity-timelocks branch from d9619f1 to 6143f89 Compare April 29, 2022 02:50
@jonathanknowles
Copy link
Member

bors try

iohk-bors bot added a commit that referenced this pull request Apr 29, 2022
@iohk-bors
Try #3249:
dd6b56d
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Apr 29, 2022 •
edited by jonathanknowles
Loading

try

Build failed:

Failure log extract:

Failures:

  src/Test/Integration/Scenario/API/Shelley/TransactionsNew.hs:2261:65: 
  1) API Specifications, NEW_SHELLEY_TRANSACTIONS, Plutus scenarios, currency
       uncaught exception: RequestException
       ClientError (Object (fromList [("code",String "redeemer_script_failure"),("message",String "I was unable to assign execution units to one of your redeemers: minting(6a14cb153ab369cb78fcb6d4c2219d8998c01215dc0152479595fe18); Its execution is failing with the following error: ValidationFailedV1 (CodecError (DeserialiseFailure 3280 \"NotEnoughSpace (0x0000004201804ce0,S {currPtr = 0x0000004201804cdf, usedBits = 6})\")) [].")]))

  To rerun use: --match "/API Specifications/NEW_SHELLEY_TRANSACTIONS/Plutus scenarios/currency/"

Randomized with seed 576944466

I'll try and reproduce locally.

UPDATE: This PASSES for me locally. I tried it 10 times, and it succeeded every time. (Regardless of the seed.)

@jonathanknowles
Replace fromIntegral with safer intCast and Finite constructor.
1b83219 
Justification:

- Using `fromIntegral` can hide unsafe integral narrowing conversions.
- Using `intCast` eliminates this class of error statically.

When creating values of `Data.Interval.Extended`, we can use the
`Finite` constructor instead of `fromIntegral`.

The following expressions are equivalent:

> Finite 1000 :: Extended Int
Finite 1000

> fromIntegral 1000 :: Extended Int
Finite 1000

> Finite 1000 == fromIntegral 1000
True
@jonathanknowles jonathanknowles force-pushed the paweljakubas/adp-1193/validity-timelocks branch from 4ee5534 to 1b83219 Compare April 29, 2022 06:59
@jonathanknowles
Rename slotInterval to scriptSlotIntervals.
99cf447 
This function takes a script and returns a list of slot intervals, rather
than just a single interval.
@jonathanknowles jonathanknowles self-requested a review April 29, 2022 07:38
@paweljakubas
Copy link
Contributor Author

bors r+

piotr-iohk
piotr-iohk reviewed Apr 29, 2022
View reviewed changes
lib/core-integration/src/Test/Integration/Scenario/API/Shelley/TransactionsNew.hs Outdated
{ "withdrawal": "self"
, "validity_interval":
{ "invalid_before": "unspecified"
, "invalid_hereafter": "unspecified"
Copy link
Contributor

Choose a reason for hiding this comment

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

The "unspecified" should be removed from the swagger then:
Screenshot from 2022-04-29 10-03-19

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Apr 29, 2022

Canceled.

@paweljakubas
Copy link
Contributor Author

bors r+

iohk-bors bot added a commit that referenced this pull request Apr 29, 2022
@iohk-bors @paweljakubas @jonathanknowles
Merge #3249
684c7a5 
3249: Validity and timelocks r=paweljakubas a=paweljakubas

<!--
Detail in a few bullet points the work accomplished in this PR.

Before you submit, don't forget to:

* Make sure the GitHub PR fields are correct:
   ✓ Set a good Title for your PR.
   ✓ Assign yourself to the PR.
   ✓ Assign one or more reviewer(s).
   ✓ Link to a Jira issue, and/or other GitHub issues or PRs.
   ✓ In the PR description delete any empty sections
     and all text commented in <!--, so that this text does not appear
     in merge commit messages.

* Don't waste reviewers' time:
   ✓ If it's a draft, select the Create Draft PR option.
   ✓ Self-review your changes to make sure nothing unexpected slipped through.

* Try to make your intent clear:
   ✓ Write a good Description that explains what this PR is meant to do.
   ✓ Jira will detect and link to this PR once created, but you can also
     link this PR in the description of the corresponding Jira ticket.
   ✓ Highlight what Testing you have done.
   ✓ Acknowledge any changes required to the Documentation.
-->


- [x] reading properly validity from request body
- [x] add checks on boundary precedence and quantity
- [x] simplify how asset quantity is called in constructTransaction (in line with decodeTransaction)
- [x] slot check
- [x] make either validity bound optional
- [x] introduce before bound to transaction context and propagate this to node's tx
- [x] extend decodeTransaction to detect txValidityInterval (with integration testing)
- [x] introducing slot computation of scripts
- [x] script vs validity check
- [x] timelock minting test on   
- [x] add unit tests in MintBurnSpec

### Comments

<!-- Additional comments, links, or screenshots to attach, if any. -->

### Issue Number
adp-1193

<!-- Reference the Jira/GitHub issue that this PR relates to, and which requirements it tackles.
  Note: Jira issues of the form ADP- will be auto-linked. -->


Co-authored-by: Pawel Jakubas <[email protected]>
Co-authored-by: IOHK <[email protected]>
Co-authored-by: Jonathan Knowles <[email protected]>
Co-authored-by: Piotr Stachyra <[email protected]>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Apr 29, 2022

Build failed:

@paweljakubas paweljakubas force-pushed the paweljakubas/adp-1193/validity-timelocks branch from 0f9cfbc to b4be703 Compare April 29, 2022 11:59
@paweljakubas
Copy link
Contributor Author

bors r+

iohk-bors bot added a commit that referenced this pull request Apr 29, 2022
@iohk-bors @paweljakubas @jonathanknowles
Merge #3249
9dd9905 
3249: Validity and timelocks r=paweljakubas a=paweljakubas

<!--
Detail in a few bullet points the work accomplished in this PR.

Before you submit, don't forget to:

* Make sure the GitHub PR fields are correct:
   ✓ Set a good Title for your PR.
   ✓ Assign yourself to the PR.
   ✓ Assign one or more reviewer(s).
   ✓ Link to a Jira issue, and/or other GitHub issues or PRs.
   ✓ In the PR description delete any empty sections
     and all text commented in <!--, so that this text does not appear
     in merge commit messages.

* Don't waste reviewers' time:
   ✓ If it's a draft, select the Create Draft PR option.
   ✓ Self-review your changes to make sure nothing unexpected slipped through.

* Try to make your intent clear:
   ✓ Write a good Description that explains what this PR is meant to do.
   ✓ Jira will detect and link to this PR once created, but you can also
     link this PR in the description of the corresponding Jira ticket.
   ✓ Highlight what Testing you have done.
   ✓ Acknowledge any changes required to the Documentation.
-->


- [x] reading properly validity from request body
- [x] add checks on boundary precedence and quantity
- [x] simplify how asset quantity is called in constructTransaction (in line with decodeTransaction)
- [x] slot check
- [x] make either validity bound optional
- [x] introduce before bound to transaction context and propagate this to node's tx
- [x] extend decodeTransaction to detect txValidityInterval (with integration testing)
- [x] introducing slot computation of scripts
- [x] script vs validity check
- [x] timelock minting test on   
- [x] add unit tests in MintBurnSpec

### Comments

<!-- Additional comments, links, or screenshots to attach, if any. -->

### Issue Number
adp-1193

<!-- Reference the Jira/GitHub issue that this PR relates to, and which requirements it tackles.
  Note: Jira issues of the form ADP- will be auto-linked. -->


Co-authored-by: Pawel Jakubas <[email protected]>
Co-authored-by: IOHK <[email protected]>
Co-authored-by: Jonathan Knowles <[email protected]>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Apr 29, 2022

Canceled.

@paweljakubas
Copy link
Contributor Author

bors r+

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Apr 29, 2022

Build succeeded:

@iohk-bors iohk-bors bot merged commit bc307c6 into master Apr 29, 2022
@iohk-bors iohk-bors bot deleted the paweljakubas/adp-1193/validity-timelocks branch April 29, 2022 17:59
WilliamKingNoel-Bot pushed a commit that referenced this pull request Apr 29, 2022
docs: Merge #3249
ad25b28
piotr-iohk pushed a commit that referenced this pull request May 4, 2022
Adjust e2e tests after API changes from #3249
58dbe4a
@piotr-iohk piotr-iohk mentioned this pull request May 4, 2022
Merged
iohk-bors bot added a commit that referenced this pull request May 6, 2022
@iohk-bors
Merge #3267
5b4790b 
3267: Update tests / pending test against ADP-1738 r=piotr-iohk a=piotr-iohk

- 58dbe4a
  Adjust e2e tests after API changes from #3249
  
- 71969cb
  Pending integration tests against ADP-1738


### Comments
Pending integration test `TRANS_NEW_CREATE_MINT_SCRIPTS` for ADP-1738
![Screenshot from 2022-05-06 13-17-16](https://user-images.githubusercontent.com/42900201/167121713-6af508af-5670-4515-9ce7-37c42eaf04dd.png)


### Issue Number

ADP-1738, ADP-1193


Co-authored-by: Piotr Stachyra <[email protected]>
iohk-bors bot added a commit that referenced this pull request May 6, 2022
@iohk-bors
Merge #3267
955c8d0 
3267: Update tests / pending test against ADP-1738 r=piotr-iohk a=piotr-iohk

- 58dbe4a
  Adjust e2e tests after API changes from #3249
  
- 71969cb
  Pending integration tests against ADP-1738


### Comments
Pending integration test `TRANS_NEW_CREATE_MINT_SCRIPTS` for ADP-1738
![Screenshot from 2022-05-06 13-17-16](https://user-images.githubusercontent.com/42900201/167121713-6af508af-5670-4515-9ce7-37c42eaf04dd.png)


### Issue Number

ADP-1738, ADP-1193


Co-authored-by: Piotr Stachyra <[email protected]>
iohk-bors bot added a commit that referenced this pull request May 6, 2022
@iohk-bors
Merge #3267
0de76e2 
3267: Update tests / pending test against ADP-1738 r=piotr-iohk a=piotr-iohk

- 58dbe4a
  Adjust e2e tests after API changes from #3249
  
- 71969cb
  Pending integration tests against ADP-1738


### Comments
Pending integration test `TRANS_NEW_CREATE_MINT_SCRIPTS` for ADP-1738
![Screenshot from 2022-05-06 13-17-16](https://user-images.githubusercontent.com/42900201/167121713-6af508af-5670-4515-9ce7-37c42eaf04dd.png)


### Issue Number

ADP-1738, ADP-1193


Co-authored-by: Piotr Stachyra <[email protected]>
iohk-bors bot added a commit that referenced this pull request May 9, 2022
@iohk-bors
Merge #3267
e8eb748 
3267: Update tests / pending test against ADP-1738 r=piotr-iohk a=piotr-iohk

- 58dbe4a
  Adjust e2e tests after API changes from #3249
  
- 71969cb
  Pending integration tests against ADP-1738


### Comments
Pending integration test `TRANS_NEW_CREATE_MINT_SCRIPTS` for ADP-1738
![Screenshot from 2022-05-06 13-17-16](https://user-images.githubusercontent.com/42900201/167121713-6af508af-5670-4515-9ce7-37c42eaf04dd.png)


### Issue Number

ADP-1738, ADP-1193


Co-authored-by: Piotr Stachyra <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@piotr-iohk piotr-iohk piotr-iohk left review comments

@jonathanknowles jonathanknowles jonathanknowles approved these changes

Assignees

@paweljakubas paweljakubas

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@paweljakubas @jonathanknowles @piotr-iohk