Page MenuHomePhabricator

Fix signrawtransaction failing when a wallet URI is specified
ClosedPublic

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

Details

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
Branch
signrawtransaction_multiwallet
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 5381
Build 8824: Bitcoin ABC Buildbot (legacy)
Build 8823: arc lint + arc unit

Event Timeline

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.

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.

This revision was automatically updated to reflect the committed changes.