Add CLI option for transaction TTL #2267

rvl · 2020-10-26T07:53:11Z

Issue Number

ADP-93 / #1840

Overview

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

Comments

Based on PR Allow deleting expired transactions from the API #2262 branch - merge that first.

KtorZ · 2020-10-26T08:00:50Z

lib/cli/src/Cardano/CLI.hs

+    <> metavar "SECONDS"
+    <> help "Time-to-live value in seconds. Default is 2 hours."
+  where
+    diffTime = fromIntegral <$> (auto :: ReadM Int)


This is slightly inconsistent with the --sync-tolerance option that is already parsing time. This option requires an explicit suffix to disambiguate the argument: 14s.

It relies on the FromText instance of SyncTolerance to do so, what about re-using that logic here too?

OK I doubt that users would particularly like having to add s when no other units are supported, but I have changed it anyway.

KtorZ · 2020-10-26T08:03:42Z

lib/cli/src/Cardano/CLI.hs

@@ -685,16 +687,17 @@ cmdWalletGetUtxoStatistics mkClient =
 -- | cardano-wallet transaction
 cmdTransaction
    :: ToJSON wallet
-    => TransactionClient
+    => Bool -- ^ Enable Shelley transaction features


May I suggest a sum-type: Era = Byron | Shelley (Goguen Voltaire) and a possible isShelley :: Era -> Bool derived from it? This would be less cryptic at the call-site when reading: cmdTransaction True client ...

OK I have added a sum type but kept it hidden in the CLI module.

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]>

KtorZ · 2020-10-27T08:27:33Z

lib/core/src/Cardano/Wallet/Orphans.hs

+            [ "Cannot parse given time duration."
+            , "Values must be given as whole positive seconds,"
+            , "and must finish with \"s\". For example: \"3s\", \"3600s\", \"42s\"."
+            ]


Alternatively, could be declared in the text-class package directly to avoid the orphan instance?

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]>

Resolves #2169

rvl · 2020-11-09T09:35:22Z

bors r+

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  - [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   2302: Fix DB migration tests r=rvl a=piotr-iohk # Issue Number  # 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 · 2020-11-09T12:48:25Z

Build failed (retrying...):

ci/hydra-build:required

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]>

piotr-iohk · 2020-11-09T14:24:58Z

@rvl:

$ cardano-wallet transaction create 1ceb45b37a94c7022837b5ca14045f11a5927c65 \
  --ttl 1s \
  --payment 1000000@addr_test1qq6m4v46wc7glh4rcdcq5u49vuqu6m8e3tp5g306chrcxt6rfvtjsyka3nefuhflvhc6pfm0kj3u7xr6c67fjhqzq3rqqtw9xt
Please enter your passphrase: ****************

Error in $['time_to_live']: parsing Quantity failed, expected Object, but encountered Number

piotr-iohk · 2020-11-09T14:31:09Z

bors r-

iohk-bors · 2020-11-09T14:31:14Z

Canceled.

rvl · 2020-11-10T08:34:53Z

Oops, thanks @piotr-iohk. It works now.

bors r+

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 · 2020-11-10T09:51:17Z

Build failed:

buildkite/cardano-wallet

Progress 6/7: cardano-wallet-core-2020.11.3
Timed out after 60 minutes.

    rollback
      Can rollback to any arbitrary known checkpoint
        +++ OK, passed 100 tests.
      Correctly re-construct tx history on rollbacks
        +++ OK, passed 400 tests:
        65.8% rolling back something
        
        34.2% Outgoing tx after point: 0
        15.0% Outgoing tx after point: 1
        11.8%

#2292

KtorZ · 2020-11-10T12:50:12Z

bors retry

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 · 2020-11-10T13:22:47Z

Build failed:

buildkite/cardano-wallet

Failures:
--
  |  
  | src/Test/Integration/Scenario/API/Shelley/Transactions.hs:642:10:
  | 1) API Specifications, SHELLEY_TRANSACTIONS, TRANS_TTL_01 - Pending transaction expiry
  | predicate failed on: SlotNo 59
  | expected expiry: SlotNo 37701
  | actual expiry: SlotNo 37760
  |  
  | To rerun use: --match "/API Specifications/SHELLEY_TRANSACTIONS/TRANS_TTL_01 - Pending transaction expiry/"
  |  
  | Randomized with seed 1007836544

#2295

rvl · 2020-11-17T01:09:08Z

bors merge

iohk-bors · 2020-11-17T01:58:39Z

Build succeeded:

#2267

rvl self-assigned this Oct 26, 2020

rvl added the ADDING FEATURE Mark a PR as adding a new feature, for auto-generated CHANGELOG label Oct 26, 2020

rvl added this to the (ADP-93) Transaction scheduler milestone Oct 26, 2020

rvl mentioned this pull request Oct 26, 2020

cardano-wallet-jormungandr --metadata option is redundant #2169

Closed

KtorZ reviewed Oct 26, 2020

View reviewed changes

rvl mentioned this pull request Oct 26, 2020

Add transaction TTL to payments API #2167

Merged

8 tasks

KtorZ approved these changes Oct 27, 2020

View reviewed changes

rvl force-pushed the rvl/1840/cli-tx-ttl branch from dae32f5 to 4e9c8a9 Compare October 27, 2020 09:52

KtorZ approved these changes Oct 30, 2020

View reviewed changes

rvl force-pushed the rvl/1840/delete-tx-expired branch from 9580b98 to 737ffd3 Compare November 2, 2020 09:24

rvl force-pushed the rvl/1840/cli-tx-ttl branch from 4e9c8a9 to 8de219b Compare November 2, 2020 09:34

rvl force-pushed the rvl/1840/delete-tx-expired branch from 737ffd3 to 866e53f Compare November 2, 2020 14:37

rvl force-pushed the rvl/1840/cli-tx-ttl branch from 8de219b to b5446ac Compare November 2, 2020 14:38

rvl force-pushed the rvl/1840/delete-tx-expired branch from 866e53f to eb74e0e Compare November 3, 2020 15:18

rvl force-pushed the rvl/1840/cli-tx-ttl branch 2 times, most recently from afed782 to 61ce155 Compare November 3, 2020 16:19

rvl force-pushed the rvl/1840/delete-tx-expired branch 2 times, most recently from f61495f to d9da868 Compare November 6, 2020 12:40

rvl force-pushed the rvl/1840/cli-tx-ttl branch from 61ce155 to 7d489fe Compare November 6, 2020 12:43

Base automatically changed from rvl/1840/delete-tx-expired to master November 6, 2020 14:42

rvl added 5 commits November 9, 2020 19:34

CLI: Remove --metadata option from cardano-wallet-jormungandr

ba6718b

Resolves #2169

CLI: Add transaction create --ttl option

7492bdd

CLI: Review rework - unblind bool

07230ed

CLI: Review rework - use same parser for ttl as for sync tolerance

c12451c

Rework - move instance FromText NominalDiffTime into text-class

265678d

rvl force-pushed the rvl/1840/cli-tx-ttl branch from 7d489fe to 265678d Compare November 9, 2020 09:34

Piotr Stachyra and others added 3 commits November 10, 2020 09:00

Integration test for tx with ttl via CLI

e84e991

Add CLI unit test for transaction create --ttl option

f9ffc9c

Fix cli create transaction --ttl json request

5a890e1

iohk-bors bot merged commit a0de704 into master Nov 17, 2020

iohk-bors bot deleted the rvl/1840/cli-tx-ttl branch November 17, 2020 01:58

rvl added a commit that referenced this pull request Nov 9, 2021

Update cardano-wallet transaction create CLI doc

8f61811

#2267

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add CLI option for transaction TTL #2267

Add CLI option for transaction TTL #2267

rvl commented Oct 26, 2020 •

edited

Loading

KtorZ Oct 26, 2020

rvl Oct 27, 2020

KtorZ Oct 26, 2020

rvl Oct 27, 2020

KtorZ Oct 27, 2020

rvl Oct 27, 2020

rvl commented Nov 9, 2020

iohk-bors bot commented Nov 9, 2020

piotr-iohk commented Nov 9, 2020

piotr-iohk commented Nov 9, 2020

iohk-bors bot commented Nov 9, 2020

rvl commented Nov 10, 2020

iohk-bors bot commented Nov 10, 2020 •

edited by Anviking

Loading

KtorZ commented Nov 10, 2020

iohk-bors bot commented Nov 10, 2020 •

edited by Anviking

Loading

rvl commented Nov 17, 2020

iohk-bors bot commented Nov 17, 2020

Add CLI option for transaction TTL #2267

Add CLI option for transaction TTL #2267

Conversation

rvl commented Oct 26, 2020 • edited Loading

Issue Number

Overview

Comments

KtorZ Oct 26, 2020

Choose a reason for hiding this comment

rvl Oct 27, 2020

Choose a reason for hiding this comment

KtorZ Oct 26, 2020

Choose a reason for hiding this comment

rvl Oct 27, 2020

Choose a reason for hiding this comment

KtorZ Oct 27, 2020

Choose a reason for hiding this comment

rvl Oct 27, 2020

Choose a reason for hiding this comment

rvl commented Nov 9, 2020

iohk-bors bot commented Nov 9, 2020

piotr-iohk commented Nov 9, 2020

piotr-iohk commented Nov 9, 2020

iohk-bors bot commented Nov 9, 2020

rvl commented Nov 10, 2020

iohk-bors bot commented Nov 10, 2020 • edited by Anviking Loading

KtorZ commented Nov 10, 2020

iohk-bors bot commented Nov 10, 2020 • edited by Anviking Loading

rvl commented Nov 17, 2020

iohk-bors bot commented Nov 17, 2020

rvl commented Oct 26, 2020 •

edited

Loading

iohk-bors bot commented Nov 10, 2020 •

edited by Anviking

Loading

iohk-bors bot commented Nov 10, 2020 •

edited by Anviking

Loading