Feat/seq bump

[elizabethengelman]

What

Add a command to allow for bumping a transaction's sequence number.

Why

After talking about tx set in our DevX meeting yesterday, we determined that maybe we should release one feature at a time, as the community finds them useful. tx edit seq-num bump seems like it would be useful right off the bat, and will be good for starting to establish the foundation for this type of subcommand.

Known limitations

This PR will need to be merged in after #1893 and may require a few updates.

The failing cargo deny advisories should be fixed in #1931

[Ifropc]

I think we should add an optional flag for seq number increment.
Sometimes (if you want to create multiple transactions) you want to increment to more than 1

[leighmcculloch]

I think the interface should change slightly by avoiding the term bump.

It looks like it bumps relatively.

The stellar network has an operation called bump sequence that bumps an account seq absolutely not relatively.

Because of the similar names and concepts on first read I thought it behaved differently than it does.

Can I suggest using a name that unambiguously implies it increases the value, by using the term increment?

Separately we probably need a sub command to set the seq num, not modify it relatively.

In regards to doing the commands piecemeal, that sgtm, but we should have a plan for all the commands so that the UX we build piecemeal is consistent and we don't use patterns for the earlier commands that don't fit the latter ones.

[leighmcculloch]

We could keep this as set and allow doing relative modifications by using + or - at the start of the number.

The upside is that there are less commands and we can reuse the pattern elsewhere, such as modifying times.

The downside would be if someone wanted to build a tx for SEP-10 that has a negative seq number, they'd have to set 0 then set -1.

[elizabethengelman]

I think the interface should change slightly by avoiding the term bump.

@leighmcculloch Good call, this totally makes sense to differentiate it from the bump seq operation. I like the idea of either using +/- or increment/decrement (could alias to inc/dec).

When I was just spiking out what using +/- would look like, I noticed that clap is treating -1 as a flag, even when that field is set to a string. The workaround I found for this so far was to pass it in as a slop arg: stellar tx edit seq-num set -- -1. Which maybe isn't ideal. I'll keep digging to see if i can find an alternative, but do you know of another way I can handle this?

Or, we could use increment/decrement or inc/dec:

• stellar tx edit sequence-number increment <INCREMENT_BY>
• stellar tx edit sequence-number decrement <DECREMENT_BY>

In regards to doing the commands piecemeal, that sgtm, but we should have a plan for all the commands so that the UX we build piecemeal is consistent and we don't use patterns for the earlier commands that don't fit the latter ones.

That makes sense! I can turn the existing tx set issue (#1643) into an "epic" and add some subtasks so we can make sure to keep track of our plan for these commands, and keep things consistent.

Separately we probably need a sub command to set the seq num, not modify it relatively.

👍 Sounds good!

[fnando]

I think we should simplify this even further. I'd call this tx edit seq-number next and avoid adding anything else until we see people are actually asking for this feature.

[elizabethengelman]

I agree with keeping things simple for now to see if people use/like this feature.

I like keeping the flexibility of allowing users to increment by more than one, as Gleb suggested above. And to me, increment or something similar makes the behavior of the command a bit clearer than next. That said, I don’t feel too strongly about it!

[fnando]

I'm looking to simplify the most common usage of sequence numbers which is getting the next one. I think it's fine to have a more generic way of setting sequence numbers (you're already doing it with increment and decrement). Personally, I feel that stellar tx new [SOMETHING] | stellar tx edit sequence-number increment is too big of a command, making it almost impossible to remember it. 🤔

[leighmcculloch]

Good call on -1 being problematic, yeah lets not consume time on making that work.

Anyone doing something advanced with seq numbers is likely going to want to set a specific value, rather than increment it. And for the more common case of updating a tx to have a new next seq, then that probably fits in a more general command like stellar tx update that calls out to the network and sets it and other information ready to submit.

[elizabethengelman]

Thanks for the feedback, all! Great conversation.

Summarizing what I'm seeing in the feedback - the different use cases that we're trying to cover are (not necessarily in this pr):

1. incrementing the seq-num by 1 (for simplicity when building tx locally)
2. incrementing the seq-num by n (when creating multiple txns)
3. allowing the user to set the seq-num to a specific positive number
4. allowing the user to set the seq-num to a specific negative number (for SEP-10)
5. incrementing based on the account's current seq-num

I wonder if going back to tx edit seq-num set <NUM> could work, if we also implement tx edit seq-num get <NUM>. I think this would allow us to handle all of the scenarios above except the 5th. Its not the simplest solution, but it is flexible. 🤷‍♀️ And then as a separate ticket we could implement tx update?

I guess I'm leaning toward a tx edit seq-num <COMMAND> structure, since we already have it in place in this PR, and it is easily extended to include more features. What do you all think? Also happy to chat more about this in our meeting today

[elizabethengelman]

I decided to put this command under stellar tx update instead of stellar tx edit, since it does feel like a different set of commands than the current edit command, that opens an editor and allows the users to edit the tx json.