Details

Reviewers

deadalnix
nakihito

Group Reviewers

Restricted Project

Commits

rABC304dfe481af7: [backport#15644] Simplify orphan processing in preparation for interruptibility

Summary

https://github.com/bitcoin/bitcoin/pull/15644/commits/9453018fdc8f02d42832374bcf1d6e3a1df02281

Partial backport of Core PR15644

Test Plan

ninja check
./test/functional/test_runner.py --extended

Diff Detail

Repository

rABC Bitcoin ABC

Branch

pr15644

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 10966
Build 20136: Default Diff Build & Tests
Build 20135: Build without Wallet
Build 20134: Build with clang-10
Build 20133: arc lint + arc unit

Event Timeline

majcosta created this revision.May 21 2020, 23:38

Herald added a reviewer: Restricted Project. · View Herald TranscriptMay 21 2020, 23:38

majcosta requested review of this revision.May 21 2020, 23:38

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

Build Bitcoin ABC Diffs / Diff Testing (build-werror) started.

Build Bitcoin ABC Diffs / Diff Testing (build-diff) started.

Build Bitcoin ABC Diffs / Diff Testing (build-without-wallet) started.

Build Bitcoin ABC Diffs / Diff Testing (build-werror) passed.

rebase

Build Bitcoin ABC Diffs / Diff Testing (build-werror) started.

Build Bitcoin ABC Diffs / Diff Testing (build-diff) started.

Build Bitcoin ABC Diffs / Diff Testing (build-without-wallet) started.

Build Bitcoin ABC Diffs / Diff Testing (build-without-wallet) passed.

Build Bitcoin ABC Diffs / Diff Testing (build-werror) passed.

Build Bitcoin ABC Diffs / Diff Testing (build-without-wallet) passed.

Build Bitcoin ABC Diffs / Diff Testing (build-diff) passed.

Harbormaster completed remote builds in B10965: Diff 20500.May 21 2020, 23:51

Build Bitcoin ABC Diffs / Diff Testing (build-diff) passed.

Harbormaster completed remote builds in B10966: Diff 20501.May 21 2020, 23:53

majcosta added a child revision: D6217: [backport#15644] [MOVEONLY] Move processing of orphan queue to ProcessOrphanTx.May 22 2020, 00:00

There is clearly some txid confusion that needs to be cleared before this can proceed.

src/net_processing.cpp
2815	Why do you need a conversion here? This is wrong, that means you got to convert some code to use txid first (or worse, this isn't not meant to be a txid and then you created a problem).
2832	Is it the hash of the txid?

This revision now requires changes to proceed.May 22 2020, 00:15

nakihito requested changes to this revision.May 22 2020, 00:31

nakihito added a subscriber: nakihito.

nakihito added inline comments.

src/net_processing.cpp
2832	This is a `TxId` not a `TxHash`, so it should still be named `orphanId`.

rebased, made a couple implicit narrowing conversions in the construction of COutPoint explicit

Build Bitcoin ABC Diffs / Diff Testing (build-werror) started.

Build Bitcoin ABC Diffs / Diff Testing (build-without-wallet) started.

Build Bitcoin ABC Diffs / Diff Testing (build-diff) started.

Build Bitcoin ABC Diffs / Diff Testing (build-werror) passed.

Build Bitcoin ABC Diffs / Diff Testing (build-without-wallet) passed.

Build Bitcoin ABC Diffs / Diff Testing (build-diff) passed.

Harbormaster completed remote builds in B10984: Diff 20536.May 22 2020, 19:24

nakihito accepted this revision.May 22 2020, 19:47

deadalnix requested changes to this revision.May 22 2020, 21:14

deadalnix added inline comments.

src/net_processing.cpp
2809 ↗	(On Diff #20536)	uint32_t(i) Also not sure about the {}. Why the change?
2862 ↗	(On Diff #20536)	dito

This revision now requires changes to proceed.May 22 2020, 21:14

majcosta added inline comments.May 22 2020, 22:01

src/net_processing.cpp
2809 ↗	(On Diff #20536)	this is what is actually happening under the hood: we're doing a narrowing conversion from size_t (unsigned long int on my machine) to uint32_t (unsigned int) the list initializer forces us to be be explicit about that and will error out if you remove the static_cast