Skip to content

[RPC][Bitcoin-TX] Add support for sequence number #7957

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
Merged
laanwj merged 3 commits into from
Jun 7, 2016
Merged

[RPC][Bitcoin-TX] Add support for sequence number #7957

laanwj merged 3 commits into from
Jun 7, 2016

Conversation

@jonasschnelli
Copy link
Contributor

@jonasschnelli jonasschnelli commented Apr 27, 2016

No description provided.

@jonasschnelli
Copy link
Contributor Author

jonasschnelli commented Apr 27, 2016

Subset PR as requested in #7865.

mruddy
mruddy reviewed Apr 28, 2016
View reviewed changes
src/rpc/rawtransaction.cpp
const UniValue& sequenceObj = find_value(o, "sequence");
if (sequenceObj.isNum())
nSequence = sequenceObj.get_int();

Copy link
Contributor

@mruddy mruddy Apr 28, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neat, this is useful. I was trying it out and think that being able to use hex is a little easier. Maybe something like this:

if (sequenceObj.isNum())
{
    nSequence = sequenceObj.get_int();
} else if (sequenceObj.isStr() /*&& IsHex(sequenceObj.get_str())*/)
{
    nSequence = strtoul(sequenceObj.get_str().c_str(), NULL, 16);
}

I commented out the IsHex check because I think it might be too restrictive (but left it there to see what you thought).
With this little patch, these work too:

createrawtransaction '[{"txid":"d5eb7e43a7cf85c5bee47a7a33c8351b8dcc013616a7c6bdc3af49538d986221","vout":0,"sequence":"0xfffffffe"}]' '{"mifzFw94bSGApmx2eWw1nzLCfEeq4cTwey":49.99}' 0
createrawtransaction '[{"txid":"d5eb7e43a7cf85c5bee47a7a33c8351b8dcc013616a7c6bdc3af49538d986221","vout":0,"sequence":"fffffffd"}]' '{"mifzFw94bSGApmx2eWw1nzLCfEeq4cTwey":49.99}' 0
createrawtransaction '[{"txid":"d5eb7e43a7cf85c5bee47a7a33c8351b8dcc013616a7c6bdc3af49538d986221","vout":0,"sequence":"f"}]' '{"mifzFw94bSGApmx2eWw1nzLCfEeq4cTwey":49.99}' 0

Copy link
Contributor Author

@jonasschnelli jonasschnelli Apr 28, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review!
Hmm... I'm not sure if we want utility function (hex->int) on machine-to-machine communication (RPC).
But no strong opinion.

Copy link
Contributor

@mruddy mruddy Apr 28, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, I was thinking of only the GUI RPC console use (that's where the hex conversion is useful to me).
If your threat model includes hostile input on this interface, then the IsHex check could be used and the length of the string could be validated to be <= 8. That would likely be sufficient input validation. Adding back IsHex makes my first and third examples invalid. The sequence value of the third would need to change to "0f" instead of just "f".

@jonasschnelli jonasschnelli mentioned this pull request Apr 28, 2016
Closed
@sipa
Copy link
Member

sipa commented Jun 2, 2016

utACK e59336f

@paveljanik
Copy link
Contributor

paveljanik commented Jun 6, 2016

utACK e59336f

@jonasschnelli
Copy link
Contributor Author

jonasschnelli commented Jun 7, 2016

Added a commit with two bitcoin-tx tests to cover the sequence number feature.

@laanwj laanwj merged commit ae357d5 into bitcoin:master Jun 7, 2016
laanwj added a commit that referenced this pull request Jun 7, 2016
@laanwj
Merge #7957: [RPC][Bitcoin-TX] Add support for sequence number
79004d4 
ae357d5 [Bitcoin-Tx] Add tests for sequence number support (Jonas Schnelli)
e59336f [bitcoin-tx] allow to set nSequence number over the in= command (Jonas Schnelli)
a946bb6 [RPC] createrawtransaction: add option to set the sequence number per input (Jonas Schnelli)
@laanwj
Copy link
Member

laanwj commented Jun 7, 2016

utACK ae357d5

This was referenced Jun 7, 2016
Merged
Merged
codablock pushed a commit to codablock/dash that referenced this pull request Dec 22, 2017
@laanwj @codablock
Merge bitcoin#7957: [RPC][Bitcoin-TX] Add support for sequence number
9af87c8 
ae357d5 [Bitcoin-Tx] Add tests for sequence number support (Jonas Schnelli)
e59336f [bitcoin-tx] allow to set nSequence number over the in= command (Jonas Schnelli)
a946bb6 [RPC] createrawtransaction: add option to set the sequence number per input (Jonas Schnelli)
@str4d str4d mentioned this pull request Mar 14, 2018
Merged
@maflcko maflcko mentioned this pull request Mar 23, 2018
Closed
zkbot added a commit to zcash/zcash that referenced this pull request Apr 13, 2018
@zkbot
Auto merge of #3086 - str4d:cli-binary-improvements-1, r=str4d
65a8f9f 
CLI binary improvements

Cherry-picked from the following upstream PRs:

- bitcoin/bitcoin#5936
- bitcoin/bitcoin#7550
- bitcoin/bitcoin#7989
- bitcoin/bitcoin#7957
- bitcoin/bitcoin#9067
- bitcoin/bitcoin#9220

Excludes any changes that affected the QT code.
@dagurval dagurval mentioned this pull request Apr 27, 2018
Merged
sickpig pushed a commit to sickpig/BitcoinUnlimited that referenced this pull request May 4, 2018
@dgenr8 @sickpig
Port XT PR BitcoinUnlimited#403: [RPC][Bitcoin-TX] Add support for se…
6c5d05a 
…quence number

bitcoin/bitcoin#7957 - [RPC][Bitcoin-TX] Add support for sequence number
bitcoin/bitcoin#8164 - [Bitcoin-Tx] fix missing test fixtures, fix 32bit atoi issue
bitcoin/bitcoin#8171 - [RPC] Fix createrawtx sequence number unsigned int parsing

[RPC][Bitcoin-TX] Add support for sequence number
@sickpig sickpig mentioned this pull request May 4, 2018
Merged
sickpig pushed a commit to sickpig/BitcoinUnlimited that referenced this pull request May 9, 2018
@dgenr8 @sickpig
Port XT PR BitcoinUnlimited#403: [RPC][Bitcoin-TX] Add support for se…
638b66e 
…quence number

bitcoin/bitcoin#7957 - [RPC][Bitcoin-TX] Add support for sequence number
bitcoin/bitcoin#8164 - [Bitcoin-Tx] fix missing test fixtures, fix 32bit atoi issue
bitcoin/bitcoin#8171 - [RPC] Fix createrawtx sequence number unsigned int parsing

[RPC][Bitcoin-TX] Add support for sequence number
andvgal pushed a commit to energicryptocurrency/gen2-energi that referenced this pull request Jan 6, 2019
@laanwj @andvgal
Merge bitcoin#7957: [RPC][Bitcoin-TX] Add support for sequence number
72bf4f6 
ae357d5 [Bitcoin-Tx] Add tests for sequence number support (Jonas Schnelli)
e59336f [bitcoin-tx] allow to set nSequence number over the in= command (Jonas Schnelli)
a946bb6 [RPC] createrawtransaction: add option to set the sequence number per input (Jonas Schnelli)
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

RPC/REST/ZMQ Utils/log/libs

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@jonasschnelli @sipa @paveljanik @laanwj @mruddy