Add DepositPreauth #958

intelliot · 2018-10-18T07:56:33Z

Add documentation of prepareTransaction, the preferred method for preparing DepositPreauth transactions.
- ripple-lib was already capable of signing DepositPreauth transactions thanks to ripple-binary-codec v0.1.15. However, this release was not tagged on GitHub, so I incremented the library's version to v0.2.0 and published it to npm and GitHub.
- I'm pinning ripple-lib to use ripple-binary-codec v0.2.0 so that we can be explicit about using updates of this dependency in the future.
Add support for parsing DepositPreauth transactions.
Allow ripple-lib to parse unrecognized transaction types, so that ripple-lib is still usable when new transaction types are added in the future.
- For unrecognized transaction types, no specification will be provided, but rawTransaction will be included.

shekenahglory · 2018-10-24T14:07:29Z

test/api-test.js

+      return checkResult(expected, 'prepare', response)
+    })
+  })
+


Would it be worthwhile to include a test for Unauthorize?

Good call. Added in cd1be9d

intelliot · 2018-10-24T22:30:27Z

@mDuo13 I exposed a method called parseAccountFlags() which takes a Flags number and returns an object naming the flags and indicating whether they are true or false.

mDuo13

I wasn't able to test out the changes, but I left some suggestions on the documentation. (I commented on the index.md file, but of course, the changes should go in the upstream ejs files first.)

mDuo13 · 2018-10-30T00:35:31Z

docs/index.md

+
+### Return Value
+
+This method returns an object with containing a key for each parsed flag. Each flag has a boolean value of `true` or `false`.


The example implies that flags that are false are omitted from the return value, or at least that flags whose value is their default value are omitted. That should be explicit. Alternatively, it would be OK (maybe more useful?) if all known flags were included (even if most of them are false).

mDuo13 · 2018-10-30T00:36:20Z

docs/index.md

+
+### Parameters
+
+This method takes one parameter, the `Flags` number to parse.


Maybe worth a note that this only works for AccountRoot Flags since the flags have different mappings on other types of objects or in transactions like AccountSet.

mDuo13 · 2018-10-30T00:37:50Z

docs/index.md

+
+Name | Type | Description
+---- | ---- | -----------
+transaction | [transaction](https://developers.ripple.com/transaction-formats.html) | The specification (JSON) of the transaction to prepare. Set `Account` to the address of the account that is creating the transaction. Common fields like `Fee`, `Flags`, and `Sequence` may be omitted; `prepareTransaction` will set them automatically.


In the rippled API documentation, we use the term "Auto-fillable fields" for fields whose values can be automatically provided, and "Common fields" for fields that apply to all transactions (but whose values cannot be automatically provided, such as Account). Let's try to match that usage here.

mDuo13 · 2018-10-30T00:42:24Z

docs/index.md

@@ -4955,6 +5060,10 @@ sign(txJSON: string, keypair: object, options: object): {signedTransaction: stri

 Sign a prepared transaction. The signed transaction must subsequently be [submitted](#submit).

+This method can sign any of [the transaction types supported by ripple-binary-codec](https://github.com/ripple/ripple-binary-codec/blob/cfcde79c19c359e9a0466d7bc3dc9a3aef47bb99/src/enums/definitions.json#L1637). When a new transaction type is added, it will be unrecognized until ripple-binary-codec is updated. An error will be thrown:


For style reasons, avoid future tense and passive voice except when talking about future events. (In this paragraph, there are two instances of future tense, one of which is appropriate and one of which isn't.) Here's a suggested rephrase:

When a new transaction type is added to the XRP Ledger, it will be unrecognized until ripple-binary-codec is updated. If you try to sign an unrecognized transaction type, this method throws an error similar to the following:

See gulpjs/gulp#2162

intelliot · 2018-10-30T19:48:02Z

This is ready for re-review

mDuo13

🥇 I was able to run the code and it works nicely. The revised wording looks good, too. (Left one nitpick you can optionally fix.)

mDuo13 · 2018-10-30T19:56:50Z

docs/index.md

@@ -4164,8 +4155,8 @@ Notably, this is the preferred method for preparing a `DepositPreauth` transacti

 Name | Type | Description
 ---- | ---- | -----------
-transaction | [transaction](https://developers.ripple.com/transaction-formats.html) | The specification (JSON) of the transaction to prepare. Set `Account` to the address of the account that is creating the transaction. Common fields like `Fee`, `Flags`, and `Sequence` may be omitted; `prepareTransaction` will set them automatically.
-instructions | [instructions](#transaction-instructions) | *Optional* Instructions for executing the transaction
+transaction | [transaction](https://developers.ripple.com/transaction-formats.html) | The specification (JSON) of the transaction to prepare. Set `Account` to the address of the account that is creating the transaction. Auto-fillable fields like `Fee`, `Flags`, and `Sequence` may be omitted; `prepareTransaction` will set them automatically.


Nit: I suggest changing the last sentence just a little more for conciseness, active voice, etc.:

You may omit auto-fillable fields like Fee, Flags, and Sequence to have them set automatically.

intelliot added 2 commits October 17, 2018 21:26

Add DepositPreauth

66462b6

Add test

a7c723d

intelliot requested a review from shekenahglory October 18, 2018 07:56

Update HISTORY

3888032

intelliot force-pushed the DepositPreauth branch from 83c794b to 3888032 Compare October 18, 2018 07:59

shekenahglory reviewed Oct 24, 2018

View reviewed changes

intelliot added 3 commits October 24, 2018 15:06

Expose parseAccountFlags()

ba52fa7

Docs: add parseAccountFlags

07fd6c2

Add unit test: DepositPreauth - Unauthorize

cd1be9d

intelliot requested a review from mDuo13 October 24, 2018 22:29

mDuo13 requested changes Oct 30, 2018

View reviewed changes

intelliot added 3 commits October 30, 2018 00:23

Update webpack from 3.11.0 to 3.12.0 and upath from 1.0.4 to 1.1.0

d40790b

To support node 10, run: yarn upgrade gulp

1d2bf4f

See gulpjs/gulp#2162

Update docs per @mDuo13 review; add example snippet

ff4e552

mDuo13 approved these changes Oct 30, 2018

View reviewed changes

intelliot added 3 commits October 30, 2018 13:02

Default to node v10 for nvm

0a4ba7c

Maintain existing behavior for getSettings

d456958

Update docs

e9229a3

intelliot requested a review from FredKSchott October 31, 2018 00:35

intelliot merged commit 4189874 into develop Oct 31, 2018

intelliot deleted the DepositPreauth branch October 31, 2018 23:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add DepositPreauth #958

Add DepositPreauth #958

intelliot commented Oct 18, 2018

shekenahglory Oct 24, 2018

intelliot Oct 24, 2018

intelliot commented Oct 24, 2018

mDuo13 left a comment

mDuo13 Oct 30, 2018

mDuo13 Oct 30, 2018

mDuo13 Oct 30, 2018

mDuo13 Oct 30, 2018

intelliot commented Oct 30, 2018

mDuo13 left a comment

mDuo13 Oct 30, 2018


		### Return Value

		This method returns an object with containing a key for each parsed flag. Each flag has a boolean value of `true` or `false`.


		### Parameters

		This method takes one parameter, the `Flags` number to parse.

		@@ -4955,6 +5060,10 @@ sign(txJSON: string, keypair: object, options: object): {signedTransaction: stri

		Sign a prepared transaction. The signed transaction must subsequently be [submitted](#submit).

		This method can sign any of [the transaction types supported by ripple-binary-codec](https://github.com/ripple/ripple-binary-codec/blob/cfcde79c19c359e9a0466d7bc3dc9a3aef47bb99/src/enums/definitions.json#L1637). When a new transaction type is added, it will be unrecognized until ripple-binary-codec is updated. An error will be thrown:

Add DepositPreauth #958

Add DepositPreauth #958

Conversation

intelliot commented Oct 18, 2018

shekenahglory Oct 24, 2018

Choose a reason for hiding this comment

intelliot Oct 24, 2018

Choose a reason for hiding this comment

intelliot commented Oct 24, 2018

mDuo13 left a comment

Choose a reason for hiding this comment

mDuo13 Oct 30, 2018

Choose a reason for hiding this comment

mDuo13 Oct 30, 2018

Choose a reason for hiding this comment

mDuo13 Oct 30, 2018

Choose a reason for hiding this comment

mDuo13 Oct 30, 2018

Choose a reason for hiding this comment

intelliot commented Oct 30, 2018

mDuo13 left a comment

Choose a reason for hiding this comment

mDuo13 Oct 30, 2018

Choose a reason for hiding this comment