Page MenuHomePhabricator

Move transaction combining from signrawtransaction to new RPC
ClosedPublic

Authored by Fabien on Oct 24 2018, 13:46.

Details

Summary

Create a combinerawtransaction RPC which accepts a json array of hex raw
transactions to combine them into one transaction. Signrawtransaction is
changed to no longer combine transactions and only accept one transaction
at a time.

Backport of core PR10571
Required for T436

Test Plan
./test/functional/test_runner.py rawtransactions signrawtransactions

Diff Detail

Repository
rABC Bitcoin ABC
Branch
split_signrawtransaction
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 3673
Build 5421: Bitcoin ABC Buildbot (legacy)
Build 5420: arc lint + arc unit

Event Timeline

deadalnix requested changes to this revision.Oct 24 2018, 23:18
deadalnix added inline comments.
src/rpc/rawtransaction.cpp
730

braces

782

Move comment on previous line

786

Move comment on previous line

790

Move comment on previous line

935

braces

This revision now requires changes to proceed.Oct 24 2018, 23:18
schancel added inline comments.
src/rpc/rawtransaction.cpp
733 ↗(On Diff #5589)

What is going on with this API? Can you point me to the docs? I doesn't look like this actually combines transactions, but only signatures?

Fabien marked an inline comment as done.Oct 31 2018, 13:51
Fabien added inline comments.
src/rpc/rawtransaction.cpp
733 ↗(On Diff #5589)

My understanding is that it targets partially signed multisig txs, and effectively combines the signatures in order to make the tx fully signed, hence spendable. I could not find any documentation apart from the help lines, but fund the functional test case being quiet self-explanatory.

src/rpc/rawtransaction.cpp
815 ↗(On Diff #5589)

So this assumes that the prevouts are all the same?

Fabien marked an inline comment as done.Nov 2 2018, 15:00
Fabien added inline comments.
src/rpc/rawtransaction.cpp
815 ↗(On Diff #5589)

My understanding is that the pk script should be the same.

jasonbcox added inline comments.
src/rpc/rawtransaction.cpp
1139 ↗(On Diff #5622)

Just a note for other reviews, because I almost missed this: This line corresponds to if (!fHashSingle || (i < mtx.vout.size())) ProduceSignature(MutableTransactionSignatureCreator(&keystore, &mtx, i, amount, nHashType), prevPubKey, sigdata); in the original PR: https://github.com/bitcoin/bitcoin/pull/10571/files It threw me off because we had some refactors on this line that made it easy to miss while scanning.

deadalnix requested changes to this revision.Nov 4 2018, 18:31

This would need a release note.

This revision now requires changes to proceed.Nov 4 2018, 18:31
src/rpc/rawtransaction.cpp
798 ↗(On Diff #5622)

size_t

Rebase, fix size_t, add an entry to the release note

This revision is now accepted and ready to land.Nov 5 2018, 23:21
This revision was automatically updated to reflect the committed changes.