Fix signrawtransaction failing when a wallet URI is specified
ClosedPublic
Actions

Authored by Fabien on Apr 4 2019, 12:16.

Details

Reviewers

florian
deadalnix
jasonbcox

Group Reviewers

Restricted Project

Commits

rSTAGINGe4eb195755c9: Fix signrawtransaction failing when a wallet URI is specified
rABCe4eb195755c9: Fix signrawtransaction failing when a wallet URI is specified

Summary

The issue has been raised by @florian.

Since D2007, the signrawtransaction RPC is routing the call to two new
RPCs, signrawtransactionwithkey and signrawtransactionwithwallet.
The routing is achieved by creating a copy of the JSON request and
forwading it to the new method.

The request URI is not copied during the process, which introduces a
regression when a node which uses multiple wallets calls
signrawtransaction: the request returns an error because the wallet is
not specified.

This diff forwards the URI to the signrawtransactionwith* RPCs and
adds a test case to check the behavior.

Test Plan

./test/functional/test_runner.py rpc_signrawtransaction

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

Fabien created this revision.Apr 4 2019, 12:16

Herald added a reviewer: Restricted Project. · View Herald TranscriptApr 4 2019, 12:16

Herald added a reviewer: deadalnix. · View Herald Transcript

Herald added a subscriber: schancel. · View Herald Transcript

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing passed.

Harbormaster completed remote builds in B5379: Diff 7940.Apr 4 2019, 12:31

Does the problem exist in Core ?

signrawtransaction is now deprecated on core, so the issue cannot happen anymore.
Up to the removal of the RPC the URI was not forwarded, and I couldn't find anything related to the issue in core, I suppose the issue has never been found or never occurred.
@florian also told me that he did some research and couldn't find anything either.

Rebase

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing passed.

Harbormaster completed remote builds in B5381: Diff 7944.Apr 4 2019, 15:32

Yeah, the issue exists in core in all actively maintained branches, except for v18.0 (removed the rpc) which should be released soon. Thanks for handling this, Fabien!

This revision is now accepted and ready to land.Apr 4 2019, 17:02

So if they have a fix, why not backport it ?

They don't have a fix. It seems they've never bumped into this problem. Core v18.0 is the only bug-free branch only because they removed the rpc entirely. I suppose we're only planning to remove that rpc in v20.0.

jasonbcox accepted this revision.Apr 5 2019, 15:56

Closed by commit rABCe4eb195755c9: Fix signrawtransaction failing when a wallet URI is specified (authored by Fabien). · Explain WhyApr 8 2019, 07:24

This revision was automatically updated to reflect the committed changes.

Fabien added a commit: rSTAGINGe4eb195755c9: Fix signrawtransaction failing when a wallet URI is specified.Sep 18 2019, 22:54