Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test turnstile violation #3964

Merged
merged 2 commits into from Apr 24, 2019

Conversation

@bitcartel
Copy link
Contributor

commented Apr 19, 2019

Adds experimental feature -developersetpoolsizezero to enable developers to test Sprout and Sapling turnstile violations on testnet and in regtest mode.

@bitcartel bitcartel requested review from str4d, daira, ebfull and zebambam Apr 19, 2019

@bitcartel bitcartel added this to Needs Prioritization in Arborist Team via automation Apr 19, 2019

@bitcartel bitcartel added this to the v2.0.5 milestone Apr 19, 2019

@garethtdavies

This comment has been minimized.

Copy link
Contributor

commented Apr 19, 2019

Trying a manual migration on testnet following listed steps, starting zcashd with -developersetpoolsizezero I get the following:

zcashd: main.cpp:3471: void FallbackSproutValuePoolBalance(CBlockIndex*, const CChainParams&): Assertion `*pindex->nChainSproutValue == chainparams.SproutValuePoolCheckpointBalance()' failed.

@bitcartel bitcartel force-pushed the bitcartel:test_turnstile_violation branch from 9ecc562 to 3cae57b Apr 19, 2019

@bitcartel

This comment has been minimized.

Copy link
Contributor Author

commented Apr 19, 2019

@garethtdavies Pushed an update to fix the issue.

@garethtdavies

This comment has been minimized.

Copy link
Contributor

commented Apr 19, 2019

Confirmed that fixes that previous issue 👍 As for the manual test, I completed the manual turnstile test with both Sprout and Sapling pools.

After starting with -developersetpoolsizezero I confirmed with getblockchaininfo and indeed both valuePools show 0:

"valuePools": [
    {
      "id": "sprout",
      "monitored": true,
      "chainValue": 0.00000000,
      "chainValueZat": 0
    },
    {
      "id": "sapling",
      "monitored": true,
      "chainValue": 0.00000000,
      "chainValueZat": 0
    }
  ]

Sprout tx: https://explorer.testnet.z.cash/tx/758c2e4b968588143f13bb4cc50b852f251912656efad0d5eeee345e02afcca7 was included in block 471087 but rejected locally:

2019-04-19 23:29:03 ERROR: ConnectBlock(): turnstile violation in Sprout shielded value pool
2019-04-19 23:29:03 Misbehaving: 198.199.112.230:18233 (0 -> 100) BAN THRESHOLD EXCEEDED
2019-04-19 23:29:03 InvalidChainFound: invalid block=00348ed7882fed4c727287476c1c04bffc469956fe4b5be9c698ce31f277dfff  height=471087  log2_work=36.576394  date=2019-04-19 23:29:01
2019-04-19 23:29:03 InvalidChainFound:  current best=006f99b1aefa225ea0d03a088f975b2b6845ae29351162f94d22da231bc6e799  height=471086  log2_work=36.576394  date=2019-04-19 23:27:24
2019-04-19 23:29:03 ERROR: ConnectTip(): ConnectBlock 00348ed7882fed4c727287476c1c04bffc469956fe4b5be9c698ce31f277dfff failed
2019-04-19 23:29:03 InvalidChainFound: invalid block=00348ed7882fed4c727287476c1c04bffc469956fe4b5be9c698ce31f277dfff  height=471087  log2_work=36.576394  date=2019-04-19 23:29:01
2019-04-19 23:29:03 InvalidChainFound:  current best=006f99b1aefa225ea0d03a088f975b2b6845ae29351162f94d22da231bc6e799  height=471086  log2_work=36.576394  date=2019-04-19 23:27:24
2019-04-19 23:29:04 ERROR: AcceptBlockHeader: block is marked invalid
2019-04-19 23:29:04 ERROR: invalid header received
2019-04-19 23:29:04 ProcessMessages(headers, 1489 bytes) FAILED peer=10
2019-04-19 23:29:09 ERROR: AcceptBlockHeader: prev block invalid

Sapling tx: https://explorer.testnet.z.cash/tx/58c0c1f828182d63d792cd5d3abadf139250c61df7f2e9bd6733d44cddb27516 was included in block 471037 but rejected locally:

2019-04-19 23:00:25 ERROR: ConnectBlock(): turnstile violation in Sapling shielded value pool
2019-04-19 23:00:25 InvalidChainFound: invalid block=03459bf2bd4307974f3bd99482a16570688eee5c94586090814b9f6bbe973f7c  height=471037  log2_work=36.576394  date=2019-04-19 23:00:23
2019-04-19 23:00:25 InvalidChainFound:  current best=000a097e2007238f4d1a88645a5fd75ed8a48cbc07de6ed718b8b65e522792e9  height=471036  log2_work=36.576394  date=2019-04-19 22:45:21
2019-04-19 23:00:25 ERROR: ConnectTip(): ConnectBlock 03459bf2bd4307974f3bd99482a16570688eee5c94586090814b9f6bbe973f7c failed
2019-04-19 23:00:25 InvalidChainFound: invalid block=03459bf2bd4307974f3bd99482a16570688eee5c94586090814b9f6bbe973f7c  height=471037  log2_work=36.576394  date=2019-04-19 23:00:23
2019-04-19 23:00:25 InvalidChainFound:  current best=000a097e2007238f4d1a88645a5fd75ed8a48cbc07de6ed718b8b65e522792e9  height=471036  log2_work=36.576394  date=2019-04-19 22:45:21
2019-04-19 23:00:31 ERROR: AcceptBlockHeader: block is marked invalid
2019-04-19 23:00:31 ERROR: invalid header received
@ebfull

ebfull approved these changes Apr 22, 2019

Copy link
Contributor

left a comment

This is great!

Show resolved Hide resolved src/chainparams.h Outdated
Show resolved Hide resolved qa/rpc-tests/turnstile.py

@bitcartel bitcartel force-pushed the bitcartel:test_turnstile_violation branch from 3cae57b to 1a1e36e Apr 22, 2019

@str4d

str4d approved these changes Apr 23, 2019

Copy link
Contributor

left a comment

utACK. Very nice 🙂

{
pindex->nChainSproutValue = 0;
pindex->nChainSaplingValue = 0;
}

This comment has been minimized.

Copy link
@str4d

str4d Apr 23, 2019

Contributor

This alters every single block, not just the chain tip, which might cause unexpected behaviour if a chain reorg were to occur while this flag was set (specifically, the reorged-to chain would be marked invalid non-deterministically). Non-blocking because I think this is sufficiently unlikely to occur, and if this happened on testnet, the developer could use the reconsiderblock RPC method to restore the main chain.

@mms710 mms710 moved this from Needs Prioritization to Protocol Backlog in Arborist Team Apr 23, 2019

@mms710 mms710 moved this from Protocol Backlog to Arborist Product Priorities in Arborist Team Apr 23, 2019

@mms710 mms710 moved this from Arborist Product Priorities to Current Sprint in Arborist Team Apr 23, 2019

@mms710 mms710 moved this from Current Sprint to In Review in Arborist Team Apr 23, 2019

@daira

daira approved these changes Apr 23, 2019

Copy link
Contributor

left a comment

ut(ACK+cov). Comments are nonblocking.

if pool['id'] == name:
assert_equal(pool['chainValue'], balance)
return
assert(False)

This comment has been minimized.

Copy link
@daira

daira Apr 23, 2019

Contributor
Suggested change
assert(False)
assert False, "pool named %r not found" % (name,)
pools = node.getblockchaininfo()['valuePools']
for pool in pools:
if pool['id'] == name:
assert_equal(pool['chainValue'], balance)

This comment has been minimized.

Copy link
@daira

daira Apr 23, 2019

Contributor
Suggested change
assert_equal(pool['chainValue'], balance)
assert_equal(pool['chainValue'], balance, message="for pool named %r" % (name,))

# Activate Sapling
self.nodes[0].generate(1)
self.sync_all()

This comment has been minimized.

Copy link
@daira

daira Apr 23, 2019

Contributor

Any reason not to simplify the test code by deleting this and activating Sapling earlier (say at block 101)?

This comment has been minimized.

Copy link
@bitcartel

bitcartel Apr 23, 2019

Author Contributor

Done along with other nits.

@bitcartel bitcartel force-pushed the bitcartel:test_turnstile_violation branch from 1a1e36e to d6b4794 Apr 23, 2019

@bitcartel

This comment has been minimized.

Copy link
Contributor Author

commented Apr 23, 2019

Addressed @daira's non-blocking review comments. 3 ACKs. @zkbot r+

@zkbot

This comment has been minimized.

Copy link
Collaborator

commented Apr 23, 2019

📌 Commit d6b4794 has been approved by bitcartel

@zkbot

This comment has been minimized.

Copy link
Collaborator

commented Apr 23, 2019

⌛️ Testing commit d6b4794 with merge e1c922d...

zkbot added a commit that referenced this pull request Apr 23, 2019

Auto merge of #3964 - bitcartel:test_turnstile_violation, r=bitcartel
Test turnstile violation

Adds experimental feature -developersetpoolsizezero to enable developers to test Sprout and Sapling turnstile violations on testnet and in regtest mode.

@bitcartel bitcartel removed the request for review from zebambam Apr 23, 2019

@bitcartel bitcartel self-assigned this Apr 23, 2019

@bitcartel bitcartel moved this from In Review to Merge Queue in Arborist Team Apr 23, 2019

@zkbot

This comment has been minimized.

Copy link
Collaborator

commented Apr 24, 2019

☀️ Test successful - pr-merge
Approved by: bitcartel
Pushing e1c922d to master...

@zkbot zkbot merged commit d6b4794 into zcash:master Apr 24, 2019

1 check passed

homu Test successful
Details

Arborist Team automation moved this from Merge Queue to Released (Merged in Master) Apr 24, 2019

This was referenced Apr 24, 2019

@milesmanley milesmanley referenced this pull request May 21, 2019

Merged

Zcash v2.0.5 merge #45

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.