Paths

Table of Contentst

Differential D8745

Convert undo.h to new serialization framework
ClosedPublic
Actions

Authored by PiRK on Dec 22 2020, 16:23.

Tags

None

Subscribers

Details

Reviewers

Group Reviewers

Restricted Project

Commits

rABC74eb60421853: Convert undo.h to new serialization framework

Summary

This is a partial backport of Core PR18021 [4/4]
https://github.com/bitcoin/bitcoin/pull/18021/commits/3c94b0039d2ca2a8c41fd6127ff5019a2afc304e

Depends on D8744

There are some important differences in the deserialization, due to differences in how the Coin object is implemented (coins.h)

Test Plan

ninja all check-all

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

PiRK created this revision.Dec 22 2020, 16:23

Herald added a reviewer: Restricted Project. · View Herald TranscriptDec 22 2020, 16:23

PiRK requested review of this revision.Dec 22 2020, 16:23

teamcity edited the summary of this revision. (Show Details)Dec 22 2020, 16:23

PiRK edited the summary of this revision. (Show Details)Dec 22 2020, 16:25

PiRK added a parent revision: D8744: Make std::vector and prevector reuse the VectorFormatter logic.

PiRK added inline comments.Dec 22 2020, 16:28

src/undo.h
57 ↗	(On Diff #26532)	I am not entirely sure about this, because we are now using a reference to a `Coin` instead of a pointer. But all the tests seem to work, so I assume that it is possible to assign a brand new `Coin` instance to `pcoin`.

Harbormaster completed remote builds in B14509: Diff 26532.Dec 22 2020, 16:51

majcosta requested changes to this revision.Dec 22 2020, 19:28

majcosta added a subscriber: majcosta.

majcosta added inline comments.

src/undo.h
31 ↗	(On Diff #26532)	can we name this `txout` like in the source material?
41 ↗	(On Diff #26532)	same for this
55 ↗	(On Diff #26532)	is the `REF(...)` still needed?
57 ↗	(On Diff #26532)	this looks fine to me

This revision now requires changes to proceed.Dec 22 2020, 19:28

PiRK added inline comments.Dec 23 2020, 10:13

src/undo.h
41 ↗	(On Diff #26532)	In `Unser`, we use a local `CTxOut` variable named `txout`. I would find it confusing to call the `Coin` object `txout` and give another name to the `CTxOut` object.
55 ↗	(On Diff #26532)	I think I can remove it, now that `pcoin` is no longer const. I ran a few tests with and without, and confirmed it is not needed.
57 ↗	(On Diff #26532)	It obviously works according to tests, but I don't understand why. My understanding was that a reference must be initialized when it is declared and once a reference is initialized to a variable, it cannot be changed to refer to a variable object.

remove unnecessary const_cast (REF()) and rename pcoin -> txout like upstream
Rename a local variable txout to out

Harbormaster completed remote builds in B14511: Diff 26538.Dec 23 2020, 13:04

majcosta accepted this revision.Dec 23 2020, 13:31

majcosta added inline comments.

src/undo.h
41 ↗	(On Diff #26532)	👍

This revision is now accepted and ready to land.Dec 23 2020, 13:31

Closed by commit rABC74eb60421853: Convert undo.h to new serialization framework (authored by Pieter Wuille <pieter.wuille@gmail.com>, committed by PiRK). · Explain WhyDec 23 2020, 14:13

This revision was automatically updated to reflect the committed changes.

PiRK added a commit: rABC74eb60421853: Convert undo.h to new serialization framework.

Revision Contents
Changeset List

Path

Size

src/

64 lines

Diff 26544

src/undo.h

Loading...