Prepare functional tests for deprecation of signrawtransaction rpc
AbandonedPublic
Actions

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

Details

Reviewers

deadalnix
jasonbcox

Group Reviewers

Restricted Project

Summary

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/test_runner.py --extended

Diff Detail

Repository

rABC Bitcoin ABC

Branch

PR10579_fix_broken_tests

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 4292
Build 6649: Bitcoin ABC Buildbot (legacy)
Build 6648: arc lint + arc unit

Event Timeline

Fabien created this revision.Dec 9 2018, 20:29

Herald added a reviewer: Restricted Project. · View Herald TranscriptDec 9 2018, 20:29

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.

Fabien mentioned this in D2007: [Part 1 of 3] Split signrawtransaction into wallet and non-wallet.Dec 9 2018, 20:31

Fabien added a child revision: D2007: [Part 1 of 3] Split signrawtransaction into wallet and non-wallet.Dec 9 2018, 20:32

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

Harbormaster completed remote builds in B4288: Diff 6309.Dec 9 2018, 20:40

Fabien mentioned this in D2008: Change signrawtransaction to signrawtransactionwithwallet in tests, add a test for legacy signrawtransaction.Dec 9 2018, 20:44

Add missing deprecated arg on node restart

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

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

Harbormaster completed remote builds in B4292: Diff 6313.Dec 9 2018, 21:03

jasonbcox accepted this revision.Dec 20 2018, 20:16

jasonbcox added a subscriber: jasonbcox.

jasonbcox added inline comments.

test/functional/mempool_reorg.py
21	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

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