Page MenuHomePhabricator

Merge #11872: [rpc] createrawtransaction: Accept sorted outputs
ClosedPublic

Authored by markblundeberg on May 20 2019, 16:11.

Details

Summary

PR11872 backport https://github.com/bitcoin/bitcoin/pull/11872/files
fac70134a rpc: Update createrawtransaction examples (MarcoFalke)
fa06dfce0 [rpc] createrawtransaction: Accept sorted outputs (MarcoFalke)
8acd25d85 rpc: Allow typeAny in RPCTypeCheck (MarcoFalke)

Pull request description:

The second parameter of the `createrawtransaction` is a dictionary of the outputs. This comes with at least two drawbacks:

* In case of duplicate keys, either of them might silently disappear, with no user feedback at all. A user needs to make other mistakes, but this could eventually lead to abnormal tx fees.
* A dictionary does not guarantee that keys are sorted. Again, a user needs to keep this in mind, as it could eventually lead to excessive tx fees.

Even though my scenario of loss-of-funds is unlikely to happen, I see it as a inconvenience that should be fixed.

Depends on D3072

Test Plan

make check
test_runner.py

Diff Detail

Repository
rABC Bitcoin ABC
Branch
createrawarray
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 5904
Build 9868: Bitcoin ABC Buildbot (legacy)
Build 9867: arc lint + arc unit

Event Timeline

deadalnix added inline comments.
doc/release-notes.md
6

What about

  • The fundrawtransaction RPC will reject the previously deprecated reserveChangeKey option.

?

src/rpc/rawtransaction.cpp
434

And this is why inline comments are bad :)

This revision is now accepted and ready to land.May 21 2019, 23:48
doc/release-notes.md
6

Yeah I dunno why the github diffs show that as a change.

Oh wait, I see-- It changed "rpc" to "RPC" for that other line!

src/rpc/rawtransaction.cpp
434

Hmm I see what you mean, it messes up the linter -- should I use /* */ instead?

mini rebase for release notes