Page MenuHomePhabricator

Prepare functional tests for deprecation of signrawtransaction rpc

Authored by Fabien on Dec 9 2018, 20:29.


Group Reviewers
Restricted Project

The changes involved for splitting signrawtransaction into
signrawtransactionwithkey and signrawtransactionwithwallet will be split
to allow for easier review, and the deprecation of signrawtransaction
will lead to multiple test failures. This diff will avoid breaking the
tests, and will be reverted when these tests are converted to the new
signrawtransactionwith* function.

Test Plan
./test/functional/ --extended

Diff Detail

rABC Bitcoin ABC
Lint OK
No Unit Test Coverage
Build Status
Buildable 4292
Build 6649: Bitcoin ABC Buildbot (legacy)
Build 6648: arc lint + arc unit

Event Timeline

Add missing deprecated arg on node restart

jasonbcox added a subscriber: jasonbcox.
jasonbcox added inline comments.

This syntax is very strange and I can't find a reference to it. I'm assuming this duplicates the inner list?

This revision is now accepted and ready to land.Dec 20 2018, 20:16
deadalnix requested changes to this revision.Jan 3 2019, 22:14

I'm not a big fan of this approach because it is completely unclear why these test require this change and not others or why this change is required at all. This change is not self contained.

Is there a reason these test cannot be changed alongside the code that require them to be changed ?

This revision now requires changes to proceed.Jan 3 2019, 22:14

signrawtransaction is being deprecated in D2007, and will break all these tests at once.

In order to avoid breaking the tests, here is the strategy:

  • This diff allow the use of the deprecated rpc
  • Then the backport (D2007) deprecates the signrawtransaction rpc, and set the ones. Tests are not broken thanks to the -deprecatedrpc option
  • The tests are fixed with the new RPC in D2008
  • This diff is reverted to remove the -deprecatedrpc option (actually part of D2008, will split pending)
  • A new test is added to check that the old api is still available through the -deprecatedrpc option, in D2009

Doing so avoid breaking the tests along the process.

Fabien requested review of this revision.Jan 4 2019, 18:15
deadalnix requested changes to this revision.Jan 5 2019, 17:09

You need to proceed as follow:

  1. Build the alternative to signrawtransaction
  2. Convert the tests to use the alternative
  3. Deprecate signrawtransaction, and add the flag to the tests that need it.
This revision now requires changes to proceed.Jan 5 2019, 17:09

Will update the workflow according to the review, this diff is no longer needed