Merge #10537: Few Minor per-utxo assert-semantics re-adds and tweak
ClosedPublic
Actions

Authored by nakihito on Jul 3 2019, 21:43.

Details

Reviewers

deadalnix
Fabien
jasonbcox
markblundeberg

Group Reviewers

Restricted Owners Package	(Owns No Changed Paths)
Restricted Project

Commits

rSTAGING28c6e32e5d03: Merge #10537: Few Minor per-utxo assert-semantics re-adds and tweak
rABC28c6e32e5d03: Merge #10537: Few Minor per-utxo assert-semantics re-adds and tweak

Summary

9417d7a33 Be much more agressive in AccessCoin docs. (Matt Corallo)
f58349ca8 Restore some assert semantics in sigop cost calculations (Matt Corallo)
3533fb4d3 Return a bool in SpendCoin to restore pre-per-utxo assert semantics (Matt Corallo)
ec1271f2b Remove useless mapNextTx lookup in CTxMemPool::TrimToSize. (Matt Corallo)

Tree-SHA512: 158a4bce063eac93e1d50709500a10a7cb1fb3271f10ed445d701852fce713e2bf0da3456088e530ab005f194ef4a2adf0c7cb23226b160cecb37a79561f29ca

Backport of Core PR10537
https://github.com/bitcoin/bitcoin/pull/10537/

Test Plan

make check
test_runner.py

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

nakihito created this revision.Jul 3 2019, 21:43

Owners added a reviewer: Restricted Owners Package.Jul 3 2019, 21:43

Herald added a reviewer: Restricted Project. · View Herald TranscriptJul 3 2019, 21:43

nakihito planned changes to this revision.Jul 3 2019, 21:43

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

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

Harbormaster completed remote builds in B6658: Diff 9973.Jul 3 2019, 21:53

coins.cpp changes were previously made here: https://reviews.bitcoinabc.org/D470
consensus/tx_verify.cpp changes were made here: https://reviews.bitcoinabc.org/D1739 (note: GetTransactionSigOpCost() was never added)
validation.cpp changes made here: https://reviews.bitcoinabc.org/D1479 and here: https://reviews.bitcoinabc.org/D471#change-8J68DPC7df8b

The changes to consensus/tx_verify.cpp need to be backported. It's not in our codebase.

This revision now requires changes to proceed.Jul 5 2019, 21:25

In D3542#84072, @Fabien wrote:

The changes to consensus/tx_verify.cpp need to be backported. It's not in our codebase.

The first change to consensus/tx_verify.cpp to GetP2SHSigOpCount() is already there. Its been turned into the function .GetOutputFor():

  const CTxOut &CCoinsViewCache::GetOutputFor(const CTxIn &input) const {
	    const Coin &coin = AccessCoin(input.prevout);
	    assert(!coin.IsSpent());
	    return coin.GetTxOut();
  }

The second change is to GetTransactionSigOpCost() but that should have been added in D1739. See https://github.com/bitcoin/bitcoin/pull/8329/files for comparison.

nakihito requested review of this revision.Sep 6 2019, 17:16

Fabien accepted this revision.Sep 9 2019, 09:16

This revision is now accepted and ready to land.Sep 9 2019, 09:16

Closed by commit rABC28c6e32e5d03: Merge #10537: Few Minor per-utxo assert-semantics re-adds and tweak (authored by Pieter Wuille <pieter.wuille@gmail.com>, committed by nakihito). · Explain WhySep 9 2019, 18:18

This revision was automatically updated to reflect the committed changes.

Nico Guiton <nico@bitframe.org> added a commit: rABC28c6e32e5d03: Merge #10537: Few Minor per-utxo assert-semantics re-adds and tweak.

Nico Guiton <nico@bitframe.org> added a commit: rSTAGING28c6e32e5d03: Merge #10537: Few Minor per-utxo assert-semantics re-adds and tweak.Sep 19 2019, 02:13