Paths

Table of Contentst

Differential D364

Update the undo operation so they return a success/error code rather than taking out parameter
ClosedPublic
Actions

Authored by deadalnix on Jul 21 2017, 18:53.

Tags

None

Subscribers

Details

Reviewers

Group Reviewers

Restricted Project

Commits

rSTAGING2f7d52036fe7: Update the undo operation so they return a success/error code rather than…
rABC2f7d52036fe7: Update the undo operation so they return a success/error code rather than…

Summary

This is backport from core. It makes the API much more understandable. See https://github.com/bitcoin/bitcoin/pull/10297/commits/db994b2de93f52c9e7bed8529ca925de5064a46f for details.

I would rather return some object that forces you to check, but we have this and it is needed for the utxo work, so be it.

Test Plan

make check
../qa/pull-tester/rpc-tests.py -extended

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

deadalnix created this revision.Jul 21 2017, 18:53

Herald added a reviewer: Restricted Project. · View Herald TranscriptJul 21 2017, 18:53

Harbormaster completed remote builds in B560: Diff 904.Jul 21 2017, 18:53

nits

Harbormaster completed remote builds in B561: Diff 905.Jul 21 2017, 19:34

CCulianu added a subscriber: CCulianu.Jul 21 2017, 19:53

CCulianu added inline comments.

src/test/undo_tests.cpp
27 ↗	(On Diff #905)	Nit: consider `++i` instead of `i++`? (as long as we're being nitty)

Rebase

Harbormaster completed remote builds in B566: Diff 913.Jul 22 2017, 14:51

freetrader requested changes to this revision.Jul 22 2017, 20:29

freetrader added inline comments.

src/test/undo_tests.cpp
39 ↗	(On Diff #913)	The returned DisconnectResult is not used. Why? If still planned, please add a TODO. If not, it might be appropriate to add a comment about why return value can be ignored here.

This revision now requires changes to proceed.Jul 22 2017, 20:29

LGTM, just have a question about why return value is ignored in one place.

deadalnix requested review of this revision.Jul 23 2017, 02:00

deadalnix edited edge metadata.

deadalnix added inline comments.

src/test/undo_tests.cpp
39 ↗	(On Diff #913)	It is a test and it is checking in the test that this does the right thing. This is not really changing anything so I'm not sure why that is blocking the diff.

freetrader accepted this revision.Jul 23 2017, 09:38

This revision is now accepted and ready to land.Jul 23 2017, 09:38

Closed by commit rABC2f7d52036fe7: Update the undo operation so they return a success/error code rather than… (authored by deadalnix, committed by deadalnix). · Explain WhyJul 23 2017, 12:35

This revision was automatically updated to reflect the committed changes.

jasonbcox mentioned this in D1971: [Part 4 of 5] Add a CChainState class to clarify internal interfaces.Apr 4 2019, 18:20

jasonbcox mentioned this in D5995: Move DisconnectResult in its own header and make it an enum class.May 7 2020, 16:45

majcosta mentioned this in D6330: [backport#16129] Make reasoning about dependencies easier by not including unused dependencies.Jun 2 2020, 21:03

Revision Contents
Changeset List

Path

Size

src/

test/

coins_tests.cpp

6 lines

7 lines

35 lines

9 lines

137 lines

Diff 919

src/test/coins_tests.cpp

Loading...

src/test/undo_tests.cpp

Loading...

src/undo.h

Loading...

src/validation.h

Loading...

src/validation.cpp

Loading...