Bitcoin 0.12 performance improvements #3262

str4d · 2018-05-14T04:54:56Z

Cherry-picked from the following upstream PRs:

Part of #2074.

str4d · 2018-05-14T04:55:27Z

@zkbot try

zkbot · 2018-05-14T04:55:33Z

⌛ Trying commit 2d921e12098fa33542cda090e8d485d0441ddf1a with merge 9217c8e09347bf3d17fb8443c5fd07dc45a20d1e...

zkbot · 2018-05-14T05:12:16Z

💔 Test failed - pr-try

str4d · 2018-05-14T06:46:32Z

Missed an indirect merge conflict.

@zkbot try

zkbot · 2018-05-14T06:46:37Z

⌛ Trying commit 127c67e with merge 8ce4bc1...

Bitcoin 0.12 performance improvements Cherry-picked from the following upstream PRs: - bitcoin/bitcoin#6918 - bitcoin/bitcoin#6932 Part of #2074.

zkbot · 2018-05-14T08:20:27Z

💔 Test failed - pr-try

str4d · 2018-05-14T23:05:47Z

Above failure was a transient failure in one of the RPC tests; all other builders passed.

daira

Zcash has no duplicate coinbase transactions; simplify the code and test accordingly.

daira · 2018-05-16T16:17:34Z

src/coins.h

+     * Return a modifiable reference to a CCoins. Assumes that no entry with the given
+     * txid exists and creates a new one. This saves a database access in the case where
+     * the coins were to be wiped out by FromTx anyway.  This should not be called with
+     * the 2 historical coinbase duplicate pairs because the new coins are marked fresh, and


There are no "2 historical coinbase duplicate pairs" in Zcash-derived chains, since we have enforced both BIP 30 and BIP 34 ("height in coinbase") since launch. This was accidentally omitted for the genesis block, but that omission can have no effect on the BIP 30/34 goal of avoiding duplicate transactions. Delete this sentence from the comment, and replace it with:

We rely on Zcash-derived block chains having no duplicate transactions, since BIP 30 and (except for the genesis block) BIP 34 have been enforced since launch. See the Zcash protocol specification, section "Bitcoin Improvement Proposals".

daira · 2018-05-16T16:20:36Z

src/main.cpp

+        // lookup to be sure the coins do not already exist otherwise we do not
+        // know whether to mark them fresh or not.  We want the duplicate coinbases
+        // before BIP30 to still be properly overwritten.
+        inputs.ModifyCoins(tx.GetHash())->FromTx(tx, nHeight);


This complication is unnecessary for Zcash-derived chains as explained above. Just call inputs.ModifyNewCoins unconditionally (instead of inputs.ModifyCoins) outside the if (!tx.IsCoinBase()) block.

Also delete the last sentence of the commit message.

daira · 2018-05-16T16:38:43Z

src/test/coins_tests.cpp

+// except the emphasis is on testing the functionality of UpdateCoins
+// random txs are created and UpdateCoins is used to update the cache stack
+// In particular it is tested that spending a duplicate coinbase tx
+// has the expected effect (the other duplicate is overwitten at all cache levels)


Simplify the test by removing the testing of duplicate coinbase transactions:

delete the last sentence of the above comment

delete the spent_a_duplicate_coinbase and duplicateids declarations

delete " unless intended to duplicate" in the comment on line 910

delete the "1/100 times create a duplicate coinbase" if body

delete the comment that mentions BIP 34 (but not the code it refers to! just the comment is inapplicable)

delete the comment starting "The test is designed" and the two code lines after it.

str4d · 2018-05-31T04:17:18Z

Addressed @daira's comments.

str4d · 2018-05-31T04:17:29Z

@zkbot try

zkbot · 2018-05-31T04:17:34Z

⌛ Trying commit 008213a with merge 03532b1...

Bitcoin 0.12 performance improvements Cherry-picked from the following upstream PRs: - bitcoin/bitcoin#6918 - bitcoin/bitcoin#6932 Part of #2074.

zkbot · 2018-05-31T05:55:13Z

☀️ Test successful - pr-try
State: approved= try=True

str4d · 2018-07-30T17:31:14Z

Rebased on master to fix merge conflict.

src/test/coins_tests.cpp

Comments addressed

When processing a new transaction, in addition to spending the Coins of its txin's it creates a new Coins for its outputs. The existing ModifyCoins function will first make sure this Coins does not already exist. It can not exist due to BIP 30, but because of that the lookup can't be cached and always has to go to the database. Since we are creating the coins to match the new tx anyway, there is no point in checking if they exist first anyway. Zcash: Modified to account for the fact that BIP 30 and BIP 34 have applied from the beginning.

Zcash: Modified to also address Zcash changes. Co-authored-by: Jack Grigg <[email protected]>

Inapplicable to Zcash because BIP 30 and BIP 34 have been applied from the beginning.

str4d · 2018-10-24T12:13:08Z

Rebased on master and address @Eirik0's comments.

daira

ut(ACK+cov) 40e4981

Eirik0

utACK

str4d · 2018-11-30T23:25:30Z

@zkbot r+

zkbot · 2018-11-30T23:25:31Z

📌 Commit 40e4981 has been approved by str4d

zkbot · 2018-11-30T23:25:36Z

⌛ Testing commit 40e4981 with merge 9cd7486...

Bitcoin 0.12 performance improvements Cherry-picked from the following upstream PRs: - bitcoin/bitcoin#6918 - bitcoin/bitcoin#6932 Part of #2074.

zkbot · 2018-12-01T01:24:47Z

☀️ Test successful - pr-merge
Approved by: str4d
Pushing 9cd7486 to master...

https://github.com/zcash/zcash/pull/3262/files

str4d added I-performance Problems and improvements with respect to performance C-upstream-port Category: Changes that are ported from the Bitcoin Core codebase. labels May 14, 2018

str4d force-pushed the 2074-perf-1 branch from 2d921e1 to 127c67e Compare May 14, 2018 06:44

zkbot added a commit that referenced this pull request May 14, 2018

Auto merge of #3262 - str4d:2074-perf-1, r=<try>

8ce4bc1

Bitcoin 0.12 performance improvements Cherry-picked from the following upstream PRs: - bitcoin/bitcoin#6918 - bitcoin/bitcoin#6932 Part of #2074.

daira previously requested changes May 16, 2018

View reviewed changes

str4d force-pushed the 2074-perf-1 branch from 127c67e to 008213a Compare May 31, 2018 04:15

zkbot added a commit that referenced this pull request May 31, 2018

Auto merge of #3262 - str4d:2074-perf-1, r=<try>

03532b1

Bitcoin 0.12 performance improvements Cherry-picked from the following upstream PRs: - bitcoin/bitcoin#6918 - bitcoin/bitcoin#6932 Part of #2074.

str4d force-pushed the 2074-perf-1 branch from 008213a to caf0a04 Compare July 30, 2018 17:30

Eirik0 reviewed Jul 31, 2018

View reviewed changes

src/test/coins_tests.cpp Outdated Show resolved Hide resolved

Eirik0 reviewed Jul 31, 2018

View reviewed changes

src/test/coins_tests.cpp Outdated Show resolved Hide resolved

Eirik0 reviewed Jul 31, 2018

View reviewed changes

src/test/coins_tests.cpp Show resolved Hide resolved

sipa and others added 5 commits October 24, 2018 19:28

Make sigcache faster and more efficient

3a78872

Evict sigcache entries that are seen in a block

e4734f2

Don't wipe the sigcache in TestBlockValidity

cf47f95

Make CCoinsViewTest behave like CCoinsViewDB

244d8a4

Zcash: Modified to also address Zcash changes. Co-authored-by: Jack Grigg <[email protected]>

morcos and others added 2 commits October 24, 2018 20:10

Add unit test for UpdateCoins

dd2d903

Remove the testing of duplicate coinbase transactions

40e4981

Inapplicable to Zcash because BIP 30 and BIP 34 have been applied from the beginning.

str4d force-pushed the 2074-perf-1 branch from caf0a04 to 40e4981 Compare October 24, 2018 12:12

str4d requested review from Eirik0 and daira October 24, 2018 12:13

daira approved these changes Nov 13, 2018

View reviewed changes

Eirik0 approved these changes Nov 29, 2018

View reviewed changes

str4d added this to the v2.0.3 milestone Nov 30, 2018

zkbot added a commit that referenced this pull request Nov 30, 2018

Auto merge of #3262 - str4d:2074-perf-1, r=str4d

9cd7486

Bitcoin 0.12 performance improvements Cherry-picked from the following upstream PRs: - bitcoin/bitcoin#6918 - bitcoin/bitcoin#6932 Part of #2074.

str4d mentioned this pull request Nov 30, 2018

Bitcoin Core 0.12.0 #2074

Open

zkbot merged commit 40e4981 into zcash:master Dec 1, 2018

bitzec added a commit to bitzec/bitzec that referenced this pull request Dec 5, 2018

Bitcoin 0.12 performance improvements zcash#3262

674f16f

https://github.com/zcash/zcash/pull/3262/files

str4d deleted the 2074-perf-1 branch February 12, 2019 22:24

Bitcoin 0.12 performance improvements #3262

Bitcoin 0.12 performance improvements #3262

Uh oh!

Conversation

str4d commented May 14, 2018

Uh oh!

str4d commented May 14, 2018

Uh oh!

zkbot commented May 14, 2018

Uh oh!

zkbot commented May 14, 2018

Uh oh!

str4d commented May 14, 2018

Uh oh!

zkbot commented May 14, 2018

Uh oh!

zkbot commented May 14, 2018

Uh oh!

str4d commented May 14, 2018

Uh oh!

daira left a comment

Choose a reason for hiding this comment

Uh oh!

daira May 16, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

daira May 16, 2018

Choose a reason for hiding this comment

Uh oh!

daira May 16, 2018

Choose a reason for hiding this comment

Uh oh!

daira May 16, 2018

Choose a reason for hiding this comment

Uh oh!

str4d commented May 31, 2018

Uh oh!

str4d commented May 31, 2018

Uh oh!

zkbot commented May 31, 2018

Uh oh!

zkbot commented May 31, 2018

Uh oh!

str4d commented Jul 30, 2018

Uh oh!

Uh oh!

Uh oh!

Uh oh!

str4d commented Oct 24, 2018

Uh oh!

daira left a comment

Choose a reason for hiding this comment

Uh oh!

Eirik0 left a comment

Choose a reason for hiding this comment

Uh oh!

str4d commented Nov 30, 2018

Uh oh!

zkbot commented Nov 30, 2018

Uh oh!

zkbot commented Nov 30, 2018

Uh oh!

zkbot commented Dec 1, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

daira May 16, 2018 •

edited

Loading