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

API/CLI support for transaction expiry #1840

Closed
rvl opened this issue Jul 1, 2020 · 0 comments
Closed

API/CLI support for transaction expiry #1840

rvl opened this issue Jul 1, 2020 · 0 comments
Assignees
@rvl
Labels
ADDING FEATURE Mark a PR as adding a new feature, for auto-generated CHANGELOG
Milestone
(ADP-93) Transact...

Comments

@rvl
Copy link
Contributor

rvl commented Jul 1, 2020 •
edited
Loading

Context

ADP-93 Transaction scheduler

Transaction TTL/expiry should be exposed in the API and the CLI.

Decision

Acceptance Criteria

  1. Must be able to set transaction expiry via payments API.
  2. Must be able to set transaction expiry via transaction create CLI.
  3. Time-to-live should be given in seconds rather than slots.
  4. Expiry must be presented as a UTC time, and should include slot info.
  5. It must be possible to clean up expired transactions from the tx history by deleting them.
  6. Specifying TTL must be optional, and a reasonable default should be used.

Development

As above

QA

  1. Integration tests are added in lib/core-integration/src/Test/Integration/Scenario/API/Shelley/Transactions.hs
    • TRANS_TTL_01
    • TRANS_TTL_02
    • TRANS_TTL_03
    • TRANS_TTL_04
    • TRANS_TTL_DELETE_01
@rvl rvl added this to the (ADP-93) Transaction scheduler milestone Jul 1, 2020
@rvl rvl mentioned this issue Sep 23, 2020
Merged
8 tasks
@rvl rvl mentioned this issue Oct 22, 2020
Merged
3 tasks
iohk-bors bot added a commit that referenced this issue Oct 26, 2020
@iohk-bors @rvl
Merge #2167
683a09c 
2167: Add transaction TTL to payments API r=rvl a=rvl

### Issue Number

ADP-93 / #1840

### Overview

- [x] Add transaction TTL to swagger spec. User provides the value in seconds.
- [x] Add transaction TTL to API types
- [x] Add TTL slot calculation to API handler function
- [x] Adjust mkStdTx to make ttl easier - it now takes the expiry slot directly.
- [x] Integration tests

### Comments

Next PR
- [ ] Add CLI option `cardano-wallet transaction create [--ttl=SECONDS]`
- [ ] Perhaps clean up the large number of function arguments and return values in transaction layer functions.


Co-authored-by: Rodney Lorrimar <[email protected]>
@rvl rvl mentioned this issue Oct 26, 2020
Merged
3 tasks
@rvl rvl self-assigned this Oct 26, 2020
@rvl rvl added the ADDING FEATURE Mark a PR as adding a new feature, for auto-generated CHANGELOG label Oct 26, 2020
iohk-bors bot added a commit that referenced this issue Oct 27, 2020
@iohk-bors @rvl
Merge #2167
5f87791 
2167: Add transaction TTL to payments API r=rvl a=rvl

### Issue Number

ADP-93 / #1840

### Overview

- [x] Add transaction TTL to swagger spec. User provides the value in seconds.
- [x] Add transaction TTL to API types
- [x] Add TTL slot calculation to API handler function
- [x] Adjust mkStdTx to make ttl easier - it now takes the expiry slot directly.
- [x] Integration tests

### Comments

Next PRs
- [x] Allow deleting expired transactions ⇒ #2262
- [x] Add CLI option `cardano-wallet transaction create [--ttl=SECONDS]` ⇒ #2267
- [ ] Perhaps clean up the large number of function arguments and return values in transaction layer functions.


Co-authored-by: Rodney Lorrimar <[email protected]>
iohk-bors bot added a commit that referenced this issue Oct 28, 2020
@iohk-bors @rvl
Merge #2167
b77d1a1 
2167: Add transaction TTL to payments API r=rvl a=rvl

### Issue Number

ADP-93 / #1840

### Overview

- [x] Add transaction TTL to swagger spec. User provides the value in seconds.
- [x] Add transaction TTL to API types
- [x] Add TTL slot calculation to API handler function
- [x] Adjust mkStdTx to make ttl easier - it now takes the expiry slot directly.
- [x] Integration tests

### Comments

Next PRs
- [x] Allow deleting expired transactions ⇒ #2262
- [x] Add CLI option `cardano-wallet transaction create [--ttl=SECONDS]` ⇒ #2267
- [ ] Perhaps clean up the large number of function arguments and return values in transaction layer functions.


Co-authored-by: Rodney Lorrimar <[email protected]>
iohk-bors bot added a commit that referenced this issue Oct 29, 2020
@iohk-bors @rvl
Merge #2167
c06c779 
2167: Add transaction TTL to payments API r=rvl a=rvl

### Issue Number

ADP-93 / #1840

### Overview

- [x] Add transaction TTL to swagger spec. User provides the value in seconds.
- [x] Add transaction TTL to API types
- [x] Add TTL slot calculation to API handler function
- [x] Adjust mkStdTx to make ttl easier - it now takes the expiry slot directly.
- [x] Integration tests

### Comments

Next PRs
- [x] Allow deleting expired transactions ⇒ #2262
- [x] Add CLI option `cardano-wallet transaction create [--ttl=SECONDS]` ⇒ #2267
- [ ] Perhaps clean up the large number of function arguments and return values in transaction layer functions.


Co-authored-by: Rodney Lorrimar <[email protected]>
iohk-bors bot added a commit that referenced this issue Nov 2, 2020
@iohk-bors @rvl @Anviking
Merge #2262 #2282
35d3cea 
2262: Allow deleting expired transactions from the API r=rvl a=rvl

### Issue Number

ADP-93 / #1840

### Overview

- [x] Using the existing transaction delete endpoint, also permit removing expired transactions.
- [x] Simplify DB model and state machine tests for this function
- [x] Integration test

### Comments

- Based on PR #2167 branch - merge that first.

2282: Increase bors timeout from 2h to 3h r=Anviking a=Anviking

# Issue Number

#2279 

# Overview

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

- [x] Increase bors timeout from 2h to 3h


# Comments

It seems hydra is often slow to schedule/run the jobs, causing ≈18% of bors r+ to fail.

It should be better to increase the timeout to 3h, than to have it fail.

This doesn't affect the timeout of buildkite or hydra themselves.

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

<!-- 
Don't forget to:

 ✓ Self-review your changes to make sure nothing unexpected slipped through
 ✓ Assign yourself to the PR
 ✓ Assign one or several reviewer(s)
 ✓ Once created, link this PR to its corresponding ticket
 ✓ Assign the PR to a corresponding milestone
 ✓ Acknowledge any changes required to the Wiki
-->


Co-authored-by: Rodney Lorrimar <[email protected]>
Co-authored-by: Johannes Lund <[email protected]>
iohk-bors bot added a commit that referenced this issue Nov 2, 2020
@iohk-bors @rvl
Merge #2262
353614e 
2262: Allow deleting expired transactions from the API r=rvl a=rvl

### Issue Number

ADP-93 / #1840

### Overview

- [x] Using the existing transaction delete endpoint, also permit removing expired transactions.
- [x] Simplify DB model and state machine tests for this function
- [x] Integration test

### Comments

- Based on PR #2167 branch - merge that first.

Co-authored-by: Rodney Lorrimar <[email protected]>
iohk-bors bot added a commit that referenced this issue Nov 4, 2020
@iohk-bors @rvl
Merge #2262
641bfb5 
2262: Allow deleting expired transactions from the API r=rvl a=rvl

### Issue Number

ADP-93 / #1840

### Overview

- [x] Using the existing transaction delete endpoint, also permit removing expired transactions.
- [x] Simplify DB model and state machine tests for this function
- [x] Integration test

### Comments

- Based on PR #2167 branch - merge that first.

Co-authored-by: Rodney Lorrimar <[email protected]>
iohk-bors bot added a commit that referenced this issue Nov 6, 2020
@iohk-bors @rvl
Merge #2262
970bebb 
2262: Allow deleting expired transactions from the API r=rvl a=rvl

### Issue Number

ADP-93 / #1840

### Overview

- [x] Using the existing transaction delete endpoint, also permit removing expired transactions.
- [x] Simplify DB model and state machine tests for this function
- [x] Integration test

### Comments

- Based on PR #2167 branch - merge that first.

Co-authored-by: Rodney Lorrimar <[email protected]>
iohk-bors bot added a commit that referenced this issue Nov 9, 2020
@iohk-bors @rvl @Anviking
Merge #2267 #2297 #2302
ee6d725 
2267: Add CLI option for transaction TTL r=rvl a=rvl

### Issue Number

ADP-93 / #1840

### Overview

- [x] Hide shelley-specific CLI options in `cardano-wallet-jormungandr` (fixes #2169)
- [x] Add option `cardano-wallet transaction create [--ttl=SECONDS]`
- [ ] Update wiki page after merging.

### Comments

- Based on PR #2262 branch - merge that first.


2297: Enable parallel integration tests in hydra r=Anviking a=Anviking

# Issue Number

ADP-466

# Overview

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

- [x] Pass in `-j 8` 
- [x] Only run CLI tests in parallel if `CI` env var isn't set
- [x] Clean up some debug printing in tests

# Comments


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

<!-- 
Don't forget to:

 ✓ Self-review your changes to make sure nothing unexpected slipped through
 ✓ Assign yourself to the PR
 ✓ Assign one or several reviewer(s)
 ✓ Once created, link this PR to its corresponding ticket
 ✓ Assign the PR to a corresponding milestone
 ✓ Acknowledge any changes required to the Wiki
-->


2302: Fix DB migration tests r=rvl a=piotr-iohk

# Issue Number

<!-- Put here a reference to the issue this PR relates to and which requirements it tackles -->


# Overview

- 098f6e5
  Fix DB migration tests



# Comments

post release


Co-authored-by: Rodney Lorrimar <[email protected]>
Co-authored-by: Johannes Lund <[email protected]>
Co-authored-by: Piotr Stachyra <[email protected]>
iohk-bors bot added a commit that referenced this issue Nov 9, 2020
@iohk-bors @rvl
Merge #2267
aca6b42 
2267: Add CLI option for transaction TTL r=rvl a=rvl

### Issue Number

ADP-93 / #1840

### Overview

- [x] Hide shelley-specific CLI options in `cardano-wallet-jormungandr` (fixes #2169)
- [x] Add option `cardano-wallet transaction create [--ttl=SECONDS]`
- [ ] Update wiki page after merging.

### Comments

- Based on PR #2262 branch - merge that first.


Co-authored-by: Rodney Lorrimar <[email protected]>
iohk-bors bot added a commit that referenced this issue Nov 10, 2020
@iohk-bors @rvl
Merge #2267
9a5b8d6 
2267: Add CLI option for transaction TTL r=rvl a=rvl

### Issue Number

ADP-93 / #1840

### Overview

- [x] Hide shelley-specific CLI options in `cardano-wallet-jormungandr` (fixes #2169)
- [x] Add option `cardano-wallet transaction create [--ttl=SECONDS]`
- [ ] Update wiki page after merging.

### Comments

- Based on PR #2262 branch - merge that first.


Co-authored-by: Rodney Lorrimar <[email protected]>
Co-authored-by: Piotr Stachyra <[email protected]>
iohk-bors bot added a commit that referenced this issue Nov 10, 2020
@iohk-bors @rvl
Merge #2267
1efdaf7 
2267: Add CLI option for transaction TTL r=rvl a=rvl

### Issue Number

ADP-93 / #1840

### Overview

- [x] Hide shelley-specific CLI options in `cardano-wallet-jormungandr` (fixes #2169)
- [x] Add option `cardano-wallet transaction create [--ttl=SECONDS]`
- [ ] Update wiki page after merging.

### Comments

- Based on PR #2262 branch - merge that first.


Co-authored-by: Rodney Lorrimar <[email protected]>
Co-authored-by: Piotr Stachyra <[email protected]>
iohk-bors bot added a commit that referenced this issue Nov 17, 2020
@iohk-bors @rvl
Merge #2267
a0de704 
2267: Add CLI option for transaction TTL r=rvl a=rvl

### Issue Number

ADP-93 / #1840

### Overview

- [x] Hide shelley-specific CLI options in `cardano-wallet-jormungandr` (fixes #2169)
- [x] Add option `cardano-wallet transaction create [--ttl=SECONDS]`
- [ ] Update wiki page after merging.

### Comments

- Based on PR #2262 branch - merge that first.


Co-authored-by: Rodney Lorrimar <[email protected]>
Co-authored-by: Piotr Stachyra <[email protected]>
@rvl rvl closed this as completed Feb 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@rvl rvl

Labels
ADDING FEATURE Mark a PR as adding a new feature, for auto-generated CHANGELOG
Projects
None yet
Milestone
(ADP-93) Transaction scheduler
Development

No branches or pull requests
1 participant
@rvl