Page MenuHomePhabricator

Partial Merge #14454: ProcessImport() cleanup (excluding witness)
ClosedPublic

Authored by nakihito on May 12 2020, 23:11.

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

nakihito created this revision.May 12 2020, 23:11
Owners added a reviewer: Restricted Owners Package.May 12 2020, 23:11
Herald added a reviewer: Restricted Project. · View Herald TranscriptMay 12 2020, 23:11
nakihito requested review of this revision.May 12 2020, 23:11
nakihito planned changes to this revision.
teamcity edited the summary of this revision. (Show Details)May 12 2020, 23:11

[Bot Message]
One or more PR numbers were detected in the summary.
Links to those PRs have been inserted into the summary for reference.

nakihito requested review of this revision.May 12 2020, 23:43
nakihito added inline comments.
src/wallet/rpcdump.cpp
1119–1123 ↗(On Diff #19974)

This change is made in PR9332.

1253–1257 ↗(On Diff #19974)

This is the reapplication of PR14679.

1269–1278 ↗(On Diff #19974)

This is removed in PR9332.

1345–1355 ↗(On Diff #19974)

This is removed in PR9332.

1396–1401 ↗(On Diff #19974)

This is changed in PR9332, but then removed in PR14454.

test/functional/wallet_importmulti.py
94–107 ↗(On Diff #19974)

This change was made in PR14454.

109–123 ↗(On Diff #19974)

This change was made in PR9332.

163–177 ↗(On Diff #19974)

This change was made in PR9332.

227–229 ↗(On Diff #19974)

These changes were made in PR14454.

251–264 ↗(On Diff #19974)

This change was made in PR9332.

nakihito edited the summary of this revision. (Show Details)May 12 2020, 23:45
nakihito edited the summary of this revision. (Show Details)
nakihito edited the summary of this revision. (Show Details)May 12 2020, 23:48

I started reviewing this but this is incredibly difficult. If the problem backport PR9332 is that some test is broken, isn't it possible to modify the PR slightly so that it doesn't break the test?

nakihito planned changes to this revision.May 13 2020, 18:31

I started reviewing this but this is incredibly difficult. If the problem backport PR9332 is that some test is broken, isn't it possible to modify the PR slightly so that it doesn't break the test?

Looks like the importmulti function test is inconsistent on the machine I was using. I have uploaded D6061 which backports only a modified PR9332 which I tested on one of the dev boxes. I'll update this one to only backport the non-witness parts of PR14454.

nakihito retitled this revision from Merge #9332 and #14454: Let wallet importmulti RPC accept labels for standard scriptPubKeys and ProcessImport() cleanup to Partial Merge #14454: ProcessImport() cleanup (excluding witness).May 13 2020, 19:17
nakihito edited the summary of this revision. (Show Details)
nakihito updated this revision to Diff 20019.May 13 2020, 19:43
nakihito edited the summary of this revision. (Show Details)

Moved PR9332 to its own patch (D6061) and rebased off it.

nakihito planned changes to this revision.May 13 2020, 19:43
nakihito requested review of this revision.May 13 2020, 19:57
deadalnix accepted this revision.May 15 2020, 13:38
This revision is now accepted and ready to land.May 15 2020, 13:38