How to set fields on Transactions

[leighmcculloch]

We don't have to do this in this PR, but we should make sequence number an option, that if isn't set uses the nxt seq num lookup, otherwise uses the value passed in. A good follow up to add later.

Originally posted by @leighmcculloch in #1551 (comment)

cc @willemneal

[leighmcculloch]

After playing with this new option, I think we should change the design.

The new option is not really usable where it is in a meaningful way without being coupled with --build-only. The reason being is that any use where the tx will be sent immediately really just needs to use the current sequence number plus one. The only need for setting a different sequence number occurs when building and later sending.

Since the option is only usable in that advanced build-only case, I think we should make this something they can do with the stellar tx ... commands. It will keep the options advertised smaller for each of the subcommands being changed here, and keeps the advanced functionality for tx modifications in the same place. Maybe as a way to simply change a sequence number stellar tx set seq, where those commands could grow to support others like stellar tx set source-account. Or maybe by using the stellar tx edit.

Originally posted by @leighmcculloch in #1702 (comment)

[leighmcculloch]

Maybe as a way to simply change a sequence number stellar tx set seq, where those commands could grow to support others like stellar tx set source-account.

Fleshing out the idea I alluded to above here's one idea to start the design process. I think given the tx structure is a tree of related fields and sub-fields, we could organise this transaction command the same.

stellar tx set source-account <SOURCE_ACCOUNT>
stellar tx set sequence-number <SEQUENCE_NUMBER>
stellar tx set fee <FEE>

stellar tx set memo none
stellar tx set memo text <MEMO_TEXT>
stellar tx set memo id <MEMO_ID>
stellar tx set memo hash <MEMO_HASH>
stellar tx set memo return <MEMO_RETURN>

stellar tx set preconditions none

stellar tx set preconditions time-bound none
stellar tx set preconditions time-bound max <MAX_TIME_BOUND>
stellar tx set preconditions time-bound min <MIN_TIME_BOUND>

stellar tx set preconditions ledger-bound none
stellar tx set preconditions ledger-bound min <MIN_LEDGER>
stellar tx set preconditions ledger-bound max <MAX_LEDGER>

stellar tx set preconditions min-seq-num none
stellar tx set preconditions min-seq-num <MIN_SEQ_NUM>
stellar tx set preconditions min-seq-age none
stellar tx set preconditions min-seq-age <MIN_SEQ_AGE>
stellar tx set preconditions min-seq-ledger-gap none
stellar tx set preconditions min-seq-ledger-gap <MIN_SEQ_LEDGER_GAP>

stellar tx set preconditions extra-signers none
stellar tx set preconditions extra-signers add <EXTRA_SIGNERS>

Some of these subcommands are pretty long, e.g. preconditions, and I think we should enable the "match abbreviated" command option that clap-rs supports. We already have it enabled in the stellar-xdr that is embedded under the 'stellar xdr' command, and it means you can type stellar xdr d, or stellar xdr dec, for stellar xdr decode. For the above commands it would allow for some abbreviation such as stellar tx set precond ... for frequent users while remaining unambiguous and self describing in their full form.

Preconditions actually have multiple ways to be constructed when there are no preconditions, and when there are only time bounds, and I think the CLI can just optimise around that, the UI doesn't need to expose those differences. The CLI can upgrade and downgrade the conditions as necessary to keep the tx as small as possible.

Thoughts? cc @sagpatil @fnando @willemneal

[janewang]

Is it necessary to follow the commands from a tree of related fields or could we drop words like preconditions as the developers don't necessarily need to know it is a precondition, only that they can set the field based on a known field name.

[leighmcculloch]

@janewang Nothing is set in stone, we could drop the tree structure, which is similar to what @willemneal did in his PR, although the PR is options oriented rather than sub-commands. The challenge with dropping no hierarchy is that some modifications of the tx have ambiguous scope. If we get rid of the stellar tx set preconditions none command, then we probably don't need the hierarchy.

[leighmcculloch]

@janewang are you suggesting something such as:

stellar tx set source-account <SOURCE_ACCOUNT>
stellar tx set sequence-number <SEQUENCE_NUMBER>
stellar tx set fee <FEE>

stellar tx set memo none
stellar tx set memo text <MEMO_TEXT>
stellar tx set memo id <MEMO_ID>
stellar tx set memo hash <MEMO_HASH>
stellar tx set memo return <MEMO_RETURN>

stellar tx set time-bound max none
stellar tx set time-bound max <MAX_TIME_BOUND>
stellar tx set time-bound min none
stellar tx set time-bound min <MIN_TIME_BOUND>

stellar tx set ledger-bound min none
stellar tx set ledger-bound min <MIN_LEDGER>
stellar tx set ledger-bound max none
stellar tx set ledger-bound max <MAX_LEDGER>

stellar tx set min-seq-num none
stellar tx set min-seq-num <MIN_SEQ_NUM>
stellar tx set min-seq-age none
stellar tx set min-seq-age <MIN_SEQ_AGE>
stellar tx set min-seq-ledger-gap none
stellar tx set min-seq-ledger-gap <MIN_SEQ_LEDGER_GAP>

stellar tx set extra-signers none
stellar tx set extra-signers add <EXTRA_SIGNERS>

Or something such as:

stellar tx set --source-account <SOURCE_ACCOUNT>
stellar tx set --sequence-number <SEQUENCE_NUMBER>
stellar tx set --fee <FEE>

stellar tx set --memo-none
stellar tx set --memo-text <MEMO_TEXT>
stellar tx set --memo-id <MEMO_ID>
stellar tx set --memo-hash <MEMO_HASH>
stellar tx set --memo-return <MEMO_RETURN>

stellar tx set --time-bound-max-none
stellar tx set --time-bound-max <MAX_TIME_BOUND>
stellar tx set --time-bound-min-none
stellar tx set --time-bound-min <MIN_TIME_BOUND>

stellar tx set --ledger-bound-min-none
stellar tx set --ledger-bound-min <MIN_LEDGER>
stellar tx set --ledger-bound-max-none
stellar tx set --ledger-bound-max <MAX_LEDGER>

stellar tx set --min-seq-num-none
stellar tx set --min-seq-num <MIN_SEQ_NUM>
stellar tx set --min-seq-age-none
stellar tx set --min-seq-age <MIN_SEQ_AGE>
stellar tx set --min-seq-ledger-gap-none
stellar tx set --min-seq-ledger-gap <MIN_SEQ_LEDGER_GAP>

stellar tx set --extra-signers-none
stellar tx set --extra-signers-add <EXTRA_SIGNERS>

@willemneal If we ignore the "preconditions none" / "--no-preconditions" case, it does seem much less ambiguous. The only option that seems a little odd at that point is the extra signers one.

[ifropc]

My 2 cents: it'd be easier to chain tx edit with flags, and I think it's a very common use case to setup multiple things at once. (--memo-text <..> --fee <...> --sequence number <...>
On the extra-signers: I agree, I think it could be confusing for people who are not very familiar with preconditions (what's the difference between --extra-signers and signing with one more key?) not sure how we can combine working easily with flags and clarity on preconditions.