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

error on startup: Assertion `nWitnessCacheSize >= nd->witnesses.size()' failed. #1378

Closed
daira opened this Issue Sep 10, 2016 · 34 comments

Comments

Projects
None yet
7 participants
@daira
Contributor

daira commented Sep 10, 2016

UpdateTip: new best=00311e4f2dae18b682086110e4ae2581857a0bb31e9eb66c127223aebffb9638  height=621  log2_work=17.079724  tx=632  date=2016-09-10 11:44:35 progress=1.000000  cache=0.2MiB(535tx)
AddToWallet 09d48c0a92b3a993ccc538555afe23388edc4c4743e473cd290a5bc6e0f6cafc  
UpdateTip: new best=0064a254e4bc19fbae2d8ef31761cfe3c2c529cd18b24d6578979e395069ba26  height=622  log2_work=17.085939  tx=634  date=2016-09-10 11:47:57 progress=1.000000  cache=0.2MiB(536tx)
AddToWallet 128e13f592d42b014181f92b8ba3ac395bd50238071a78c59dc6eb733b179f89  
zcashd: wallet/wallet.cpp:605: void CWallet::IncrementNoteWitnesses(const CBlockIndex*, const CBlock*, ZCIncrementalMerkleTree): Assertion `nWitnessCacheSize >= nd->witnesses.size()' failed.
Aborted

This is after my VM crashed and restarted; don't know whether that's relevant.

@daira

This comment has been minimized.

Show comment
Hide comment
@daira

daira Sep 10, 2016

Contributor

After the above error, restarting zcashd now fails consistently at an earlier tx:

LoadBlockIndexDB: last block file = 0
LoadBlockIndexDB: last block file info: CBlockFileInfo(blocks=637, size=1123853, heights=0...636, time=2011-02-02...2016-09-10)
Checking all blk files are present...
LoadBlockIndexDB: transaction index disabled
LoadBlockIndexDB: hashBestChain=02213854cdbaa97390f48b0426315b23aed512bce8b4af84183de11dd5cdb7ca height=205 date=2016-09-09 21:24:26 progress=1.000000
init message: Verifying blocks...
Verifying last 205 blocks at level 3
No coin database inconsistencies in last 205 blocks (205 transactions)
 block index             733ms
init message: Loading wallet...
nFileVersion = 1000000
Keys: 120 plaintext, 0 encrypted, 120 w/ metadata, 120 total
ZKeys: 1 plaintext, -- encrypted, 1 w/metadata, 1 total
 wallet                  681ms
init message: Activating best chain...
UpdateTip: new best=0241a299f2bbdd06f4f981bb34ad45083bce3328dc82cec0a09666edc4836502  height=206  log2_work=12.817583  tx=207  date=2016-09-09 21:25:18 progress=1.000000  cache=0.0MiB(1tx)
zcashd: wallet/wallet.cpp:605: void CWallet::IncrementNoteWitnesses(const CBlockIndex*, const CBlock*, ZCIncrementalMerkleTree): Assertion `nWitnessCacheSize >= nd->witnesses.size()' failed.
Aborted
Contributor

daira commented Sep 10, 2016

After the above error, restarting zcashd now fails consistently at an earlier tx:

LoadBlockIndexDB: last block file = 0
LoadBlockIndexDB: last block file info: CBlockFileInfo(blocks=637, size=1123853, heights=0...636, time=2011-02-02...2016-09-10)
Checking all blk files are present...
LoadBlockIndexDB: transaction index disabled
LoadBlockIndexDB: hashBestChain=02213854cdbaa97390f48b0426315b23aed512bce8b4af84183de11dd5cdb7ca height=205 date=2016-09-09 21:24:26 progress=1.000000
init message: Verifying blocks...
Verifying last 205 blocks at level 3
No coin database inconsistencies in last 205 blocks (205 transactions)
 block index             733ms
init message: Loading wallet...
nFileVersion = 1000000
Keys: 120 plaintext, 0 encrypted, 120 w/ metadata, 120 total
ZKeys: 1 plaintext, -- encrypted, 1 w/metadata, 1 total
 wallet                  681ms
init message: Activating best chain...
UpdateTip: new best=0241a299f2bbdd06f4f981bb34ad45083bce3328dc82cec0a09666edc4836502  height=206  log2_work=12.817583  tx=207  date=2016-09-09 21:25:18 progress=1.000000  cache=0.0MiB(1tx)
zcashd: wallet/wallet.cpp:605: void CWallet::IncrementNoteWitnesses(const CBlockIndex*, const CBlock*, ZCIncrementalMerkleTree): Assertion `nWitnessCacheSize >= nd->witnesses.size()' failed.
Aborted
@str4d

This comment has been minimized.

Show comment
Hide comment
@str4d

str4d Sep 10, 2016

Contributor

If the VM crashed part-way through the part of IncrementNoteWitnesses() that writes to disk, it is possible that some or all of the witnesses were persisted but nWitnessCacheSize was not. In that case, it is possible that one or more of the persisted witness lists is greater than the known cache size, breaking an expected invariant.

One possible fix is to just write out the cache size first; then any crash will ensure that this invariant still holds. However, this doesn't address the fact that the witness lists and cache size can get out-of-sync; in fact, reversing the write order just makes it harder to detect. I think we should replace the assertion with a conditional call to RebuildNoteWitnesses(), which we will be adding anyway for #1302.

Contributor

str4d commented Sep 10, 2016

If the VM crashed part-way through the part of IncrementNoteWitnesses() that writes to disk, it is possible that some or all of the witnesses were persisted but nWitnessCacheSize was not. In that case, it is possible that one or more of the persisted witness lists is greater than the known cache size, breaking an expected invariant.

One possible fix is to just write out the cache size first; then any crash will ensure that this invariant still holds. However, this doesn't address the fact that the witness lists and cache size can get out-of-sync; in fact, reversing the write order just makes it harder to detect. I think we should replace the assertion with a conditional call to RebuildNoteWitnesses(), which we will be adding anyway for #1302.

@bitcartel

This comment has been minimized.

Show comment
Hide comment
@bitcartel

bitcartel Sep 10, 2016

Contributor

Mobile so will keep this short: storing witness should be atomic, and bdb should recover from transaction logs.

Contributor

bitcartel commented Sep 10, 2016

Mobile so will keep this short: storing witness should be atomic, and bdb should recover from transaction logs.

@daira

This comment has been minimized.

Show comment
Hide comment
@daira

daira Sep 11, 2016

Contributor

I'm just going to put this link here :-p #1233 (comment)

Contributor

daira commented Sep 11, 2016

I'm just going to put this link here :-p #1233 (comment)

@bitcartel

This comment has been minimized.

Show comment
Hide comment
@bitcartel
Contributor

bitcartel commented Sep 13, 2016

@bitcartel bitcartel added the bug label Sep 13, 2016

@daira

This comment has been minimized.

Show comment
Hide comment
@daira

daira Sep 13, 2016

Contributor

BTW I have a copy of the testnet3 directory if you want it.

Contributor

daira commented Sep 13, 2016

BTW I have a copy of the testnet3 directory if you want it.

@str4d

This comment has been minimized.

Show comment
Hide comment
@str4d

str4d Sep 13, 2016

Contributor

Not sure why I didn't use the transactions feature of the database the first time 😝 I've implemented that in #1384. I have no idea how I could go about testing that it fixes this issue though, since engineering such a crash is well outside the unit testing framework we have, and I'm not even sure how (or if) we could reliably engineer one with the Bitcoin RPC testing framework.

Contributor

str4d commented Sep 13, 2016

Not sure why I didn't use the transactions feature of the database the first time 😝 I've implemented that in #1384. I have no idea how I could go about testing that it fixes this issue though, since engineering such a crash is well outside the unit testing framework we have, and I'm not even sure how (or if) we could reliably engineer one with the Bitcoin RPC testing framework.

@nathan-at-least

This comment has been minimized.

Show comment
Hide comment
@nathan-at-least

nathan-at-least Sep 13, 2016

Contributor

Can we recreate this assertion failure repeatably with a newly introduced automated test?

Contributor

nathan-at-least commented Sep 13, 2016

Can we recreate this assertion failure repeatably with a newly introduced automated test?

@nathan-at-least

This comment has been minimized.

Show comment
Hide comment
@nathan-at-least

nathan-at-least Sep 13, 2016

Contributor

If testing this is too difficult, are there any short term work-around instructions we can give to users to help them avoid destroying money?

Contributor

nathan-at-least commented Sep 13, 2016

If testing this is too difficult, are there any short term work-around instructions we can give to users to help them avoid destroying money?

@bitcartel

This comment has been minimized.

Show comment
Hide comment
@bitcartel

bitcartel Sep 14, 2016

Contributor

@daira Do you remember what commands you ran before you saw the problem? @str4d Perhaps we can try to replicate and make sure the patched version does not have the problem?

Contributor

bitcartel commented Sep 14, 2016

@daira Do you remember what commands you ran before you saw the problem? @str4d Perhaps we can try to replicate and make sure the patched version does not have the problem?

@str4d

This comment has been minimized.

Show comment
Hide comment
@str4d

str4d Sep 15, 2016

Contributor

My guess is that this is related to #1400, but I haven't done any testing yet to confirm.

Contributor

str4d commented Sep 15, 2016

My guess is that this is related to #1400, but I haven't done any testing yet to confirm.

@daira

This comment has been minimized.

Show comment
Hide comment
@daira

daira Sep 15, 2016

Contributor

The node was just mining in the background (on three threads) when the VM crashed; any prior commands had long completed. (That VM is unstable in general, so I have no particular reason to believe the VM crash was related to running zcashd.)

Contributor

daira commented Sep 15, 2016

The node was just mining in the background (on three threads) when the VM crashed; any prior commands had long completed. (That VM is unstable in general, so I have no particular reason to believe the VM crash was related to running zcashd.)

@daira

This comment has been minimized.

Show comment
Hide comment
@daira

daira Sep 15, 2016

Contributor

I suspect that it can be reproduced by triggering an exit part-way through the code that is now in a transaction.

Contributor

daira commented Sep 15, 2016

I suspect that it can be reproduced by triggering an exit part-way through the code that is now in a transaction.

@str4d str4d self-assigned this Sep 19, 2016

zkbot pushed a commit that referenced this issue Sep 27, 2016

zkbot
Auto merge of #1384 - str4d:1378-atomic-witness-storage, r=str4d
Write note witness cache atomically to disk to avoid corruption

Closes #1378

@str4d str4d added the has PR label Sep 27, 2016

@str4d

This comment has been minimized.

Show comment
Hide comment
@str4d

str4d Sep 27, 2016

Contributor

This will be closed by #1384; we can re-open if the issue persists in beta 2.

Contributor

str4d commented Sep 27, 2016

This will be closed by #1384; we can re-open if the issue persists in beta 2.

str4d added a commit to str4d/zcash that referenced this issue Sep 27, 2016

zkbot pushed a commit that referenced this issue Sep 28, 2016

zkbot
Auto merge of #1384 - str4d:1378-atomic-witness-storage, r=ebfull
Write note witness cache atomically to disk to avoid corruption

Closes #1378

@zkbot zkbot closed this in #1384 Sep 28, 2016

@ngreatorex

This comment has been minimized.

Show comment
Hide comment
@ngreatorex

ngreatorex Oct 8, 2016

Hi,

I've had this error occur running 1.0.0-beta2. As I understand it from the above, I shouldn't have. I'm happy to send a copy of my testnet3 directory if you want it.

I had zcashd initially running in the background. I came back to find out that it had crashed, but with nothing useful in the logs. On restarting zcashd, I got the following:

neil@hyperbuntu:~$ ./zcash/src/zcashd -port=19233 -rpcport=19232 -printtoconsole

Bitcoin version v1.0.0-beta2-56ffeae (2016-10-05 04:16:21 -0500)
Using OpenSSL version OpenSSL 1.1.0b  26 Sep 2016
Using BerkeleyDB version Berkeley DB 5.3.28: (September  9, 2013)
Default data directory /home/neil/.zcash
Using data directory /home/neil/.zcash/testnet3
Using config file /home/neil/.zcash/zcash.conf
Using at most 125 connections (1024 file descriptors available)
Using 0 threads for script verification
Loading verifying key from /home/neil/.zcash-params/beta2-verifying.key
Loaded verifying key in 0.000019s seconds.
Binding RPC on address ::1 port 19232 (IPv4+IPv6 bind any: 0)
Binding RPC on address 127.0.0.1 port 19232 (IPv4+IPv6 bind any: 0)
Using wallet wallet.dat
init message: Verifying wallet...
CDBEnv::Open: LogDir=/home/neil/.zcash/testnet3/database ErrorFile=/home/neil/.zcash/testnet3/db.log
Bound to [::]:19233
Bound to 0.0.0.0:19233
Block index database configuration:
* Using 1000 max open files
* Compression is enabled
Cache configuration:
* Max cache setting possible 16384MiB
* Using 2.0MiB for block index database
* Using 32.5MiB for chain state database
* Using 65.5MiB for in-memory UTXO set
init message: Loading block index...
Opening LevelDB in /home/neil/.zcash/testnet3/blocks/index
scheduler thread start
Opened LevelDB successfully
Opening LevelDB in /home/neil/.zcash/testnet3/chainstate
Opened LevelDB successfully
LoadBlockIndexDB: last block file = 0
LoadBlockIndexDB: last block file info: CBlockFileInfo(blocks=2253, size=3961347, heights=0...2251, time=2011-02-02...2016-10-08)
Checking all blk files are present...
LoadBlockIndexDB: transaction index disabled
LoadBlockIndexDB: address index disabled
LoadBlockIndexDB: timestamp index disabled
LoadBlockIndexDB: spent index disabled
LoadBlockIndexDB: hashBestChain=002bcd23547a3c42ae27f88dfb904ba1161bdd6cb41db70e38d70cd14ddf257e height=1710 date=2016-10-07 21:46:23 progress=1.000000
init message: Verifying blocks...
Verifying last 288 blocks at level 3
No coin database inconsistencies in last 289 blocks (312 transactions)
 block index            1126ms
init message: Loading wallet...
nFileVersion = 1000001
Keys: 114 plaintext, 0 encrypted, 114 w/ metadata, 114 total
ZKeys: 1 plaintext, 0 encrypted, 1 w/metadata, 1 total
 wallet                  175ms
init message: Activating best chain...
UpdateTip: new best=001163d7c2b2ed779d0b490859cb025527058bb5212ecf73b64f81c71cf756d1  height=1711  log2_work=19.939977  tx=1811  date=2016-10-07 21:51:52 progress=1.000000  cache=0.0MiB(1tx)
zcashd: wallet/wallet.cpp:648: void CWallet::IncrementNoteWitnesses(const CBlockIndex*, const CBlock*, ZCIncrementalMerkleTree): Assertion `nWitnessCacheSize >= nd->witnesses.size()' failed.
Aborted (core dumped)

ngreatorex commented Oct 8, 2016

Hi,

I've had this error occur running 1.0.0-beta2. As I understand it from the above, I shouldn't have. I'm happy to send a copy of my testnet3 directory if you want it.

I had zcashd initially running in the background. I came back to find out that it had crashed, but with nothing useful in the logs. On restarting zcashd, I got the following:

neil@hyperbuntu:~$ ./zcash/src/zcashd -port=19233 -rpcport=19232 -printtoconsole

Bitcoin version v1.0.0-beta2-56ffeae (2016-10-05 04:16:21 -0500)
Using OpenSSL version OpenSSL 1.1.0b  26 Sep 2016
Using BerkeleyDB version Berkeley DB 5.3.28: (September  9, 2013)
Default data directory /home/neil/.zcash
Using data directory /home/neil/.zcash/testnet3
Using config file /home/neil/.zcash/zcash.conf
Using at most 125 connections (1024 file descriptors available)
Using 0 threads for script verification
Loading verifying key from /home/neil/.zcash-params/beta2-verifying.key
Loaded verifying key in 0.000019s seconds.
Binding RPC on address ::1 port 19232 (IPv4+IPv6 bind any: 0)
Binding RPC on address 127.0.0.1 port 19232 (IPv4+IPv6 bind any: 0)
Using wallet wallet.dat
init message: Verifying wallet...
CDBEnv::Open: LogDir=/home/neil/.zcash/testnet3/database ErrorFile=/home/neil/.zcash/testnet3/db.log
Bound to [::]:19233
Bound to 0.0.0.0:19233
Block index database configuration:
* Using 1000 max open files
* Compression is enabled
Cache configuration:
* Max cache setting possible 16384MiB
* Using 2.0MiB for block index database
* Using 32.5MiB for chain state database
* Using 65.5MiB for in-memory UTXO set
init message: Loading block index...
Opening LevelDB in /home/neil/.zcash/testnet3/blocks/index
scheduler thread start
Opened LevelDB successfully
Opening LevelDB in /home/neil/.zcash/testnet3/chainstate
Opened LevelDB successfully
LoadBlockIndexDB: last block file = 0
LoadBlockIndexDB: last block file info: CBlockFileInfo(blocks=2253, size=3961347, heights=0...2251, time=2011-02-02...2016-10-08)
Checking all blk files are present...
LoadBlockIndexDB: transaction index disabled
LoadBlockIndexDB: address index disabled
LoadBlockIndexDB: timestamp index disabled
LoadBlockIndexDB: spent index disabled
LoadBlockIndexDB: hashBestChain=002bcd23547a3c42ae27f88dfb904ba1161bdd6cb41db70e38d70cd14ddf257e height=1710 date=2016-10-07 21:46:23 progress=1.000000
init message: Verifying blocks...
Verifying last 288 blocks at level 3
No coin database inconsistencies in last 289 blocks (312 transactions)
 block index            1126ms
init message: Loading wallet...
nFileVersion = 1000001
Keys: 114 plaintext, 0 encrypted, 114 w/ metadata, 114 total
ZKeys: 1 plaintext, 0 encrypted, 1 w/metadata, 1 total
 wallet                  175ms
init message: Activating best chain...
UpdateTip: new best=001163d7c2b2ed779d0b490859cb025527058bb5212ecf73b64f81c71cf756d1  height=1711  log2_work=19.939977  tx=1811  date=2016-10-07 21:51:52 progress=1.000000  cache=0.0MiB(1tx)
zcashd: wallet/wallet.cpp:648: void CWallet::IncrementNoteWitnesses(const CBlockIndex*, const CBlock*, ZCIncrementalMerkleTree): Assertion `nWitnessCacheSize >= nd->witnesses.size()' failed.
Aborted (core dumped)
@str4d

This comment has been minimized.

Show comment
Hide comment
@str4d

str4d Oct 15, 2016

Contributor

If I can't identify the cause, worst case is we just remove the assertion, and adjust the method to pop off enough from the rear to bring it back within the bounds. That would prevent the node crashing, but would sometimes mean that notes could not be spent, because their witnesses would fail to validate on other nodes (and elsewhere within the local node itself). The fix would be a -reindex.

Contributor

str4d commented Oct 15, 2016

If I can't identify the cause, worst case is we just remove the assertion, and adjust the method to pop off enough from the rear to bring it back within the bounds. That would prevent the node crashing, but would sometimes mean that notes could not be spent, because their witnesses would fail to validate on other nodes (and elsewhere within the local node itself). The fix would be a -reindex.

@str4d

This comment has been minimized.

Show comment
Hide comment
@str4d

str4d Oct 15, 2016

Contributor

I've noticed that in IncrementNoteWitnesses, when we find a note in the current block that is ours, we witness it - but we don't ensure that we have never witnessed it before. So it's possible that somehow IncrementNoteWitnesses is being called twice for the same block, and so two starting witnesses are being added for the same note. If this is the cause of the assertion failure, it means two things:

  • For the notes causing the crash, their witness caches are fine
  • For all other notes, their witness caches will be corrupted (because they will have witnessed the same block twice).

I'm going to think over how I can leverage the cached witness height added in #1526 (when it is merged) to improve IncrementNoteWitnesses.

Contributor

str4d commented Oct 15, 2016

I've noticed that in IncrementNoteWitnesses, when we find a note in the current block that is ours, we witness it - but we don't ensure that we have never witnessed it before. So it's possible that somehow IncrementNoteWitnesses is being called twice for the same block, and so two starting witnesses are being added for the same note. If this is the cause of the assertion failure, it means two things:

  • For the notes causing the crash, their witness caches are fine
  • For all other notes, their witness caches will be corrupted (because they will have witnessed the same block twice).

I'm going to think over how I can leverage the cached witness height added in #1526 (when it is merged) to improve IncrementNoteWitnesses.

@str4d

This comment has been minimized.

Show comment
Hide comment
@str4d

str4d Oct 15, 2016

Contributor

Another, more useful workaround than removing the assert would be to change it to a conditional, and print an error message to the log informing users that they will probably need to -reindex before sending any shielded transactions.

Contributor

str4d commented Oct 15, 2016

Another, more useful workaround than removing the assert would be to change it to a conditional, and print an error message to the log informing users that they will probably need to -reindex before sending any shielded transactions.

@daira

This comment has been minimized.

Show comment
Hide comment
@daira

daira Oct 15, 2016

Contributor

If there's an assertion of an invariant being violated, the easiest way to debug it is to make it at every change to the affected variables, so that you can see precisely where it breaks.

Contributor

daira commented Oct 15, 2016

If there's an assertion of an invariant being violated, the easiest way to debug it is to make it at every change to the affected variables, so that you can see precisely where it breaks.

@str4d

This comment has been minimized.

Show comment
Hide comment
@str4d

str4d Oct 15, 2016

Contributor

Agreed, but all we have currently is the post-break state. If that is the proposal, then we'd want to implement it in RC 1 and wait for more users to report crashes, which means that a non-crashing build would need to be released later (RC 2?).

Contributor

str4d commented Oct 15, 2016

Agreed, but all we have currently is the post-break state. If that is the proposal, then we'd want to implement it in RC 1 and wait for more users to report crashes, which means that a non-crashing build would need to be released later (RC 2?).

@daira

This comment has been minimized.

Show comment
Hide comment
@daira

daira Oct 15, 2016

Contributor

I was thinking we might be able to trigger it just by playing around with transfers on the testnet ourselves. Also, adding the finer-grained assertions sometimes makes the bug jump out at you.

Contributor

daira commented Oct 15, 2016

I was thinking we might be able to trigger it just by playing around with transfers on the testnet ourselves. Also, adding the finer-grained assertions sometimes makes the bug jump out at you.

@str4d

This comment has been minimized.

Show comment
Hide comment
@str4d

str4d Oct 16, 2016

Contributor

I've added more assertions in https://github.com/str4d/zcash/tree/1378-witness-cache-assertion-failure so anyone who wants to try and find the bug can play with that branch.

Contributor

str4d commented Oct 16, 2016

I've added more assertions in https://github.com/str4d/zcash/tree/1378-witness-cache-assertion-failure so anyone who wants to try and find the bug can play with that branch.

@vaklinov

This comment has been minimized.

Show comment
Hide comment
@vaklinov

vaklinov Oct 16, 2016

I also have this problem with ZCash 1.0.0-beta2. I have a wallet with 3 Z addresses and several transactions were sent from it. At one point zcashd crashed. It still crashes when I delete the blockchain and have it downloaded again... blcokchain is never synchronized 100%. zcashd crashes with:

zcashd: wallet/wallet.cpp:648: void CWallet::IncrementNoteWitnesses(const CBlockIndex*, const CBlock*, ZCIncrementalMerkleTree): Assertion `nWitnessCacheSize >= nd->witnesses.size()' failed.

Wallet is: wallet.dat.zip

vaklinov commented Oct 16, 2016

I also have this problem with ZCash 1.0.0-beta2. I have a wallet with 3 Z addresses and several transactions were sent from it. At one point zcashd crashed. It still crashes when I delete the blockchain and have it downloaded again... blcokchain is never synchronized 100%. zcashd crashes with:

zcashd: wallet/wallet.cpp:648: void CWallet::IncrementNoteWitnesses(const CBlockIndex*, const CBlock*, ZCIncrementalMerkleTree): Assertion `nWitnessCacheSize >= nd->witnesses.size()' failed.

Wallet is: wallet.dat.zip

zkbot pushed a commit that referenced this issue Oct 16, 2016

zkbot
Auto merge of #1542 - daira:1378-witness-cache-assertion-failure, r=<…
…try>

[WIP] 1378 witness cache assertion failure

Do not merge, for testing only. refs #1378

zkbot pushed a commit that referenced this issue Oct 16, 2016

zkbot
Auto merge of #1542 - daira:1378-witness-cache-assertion-failure, r=<…
…try>

[WIP] 1378 witness cache assertion failure

Do not merge, for testing only. refs #1378

zkbot pushed a commit that referenced this issue Oct 17, 2016

zkbot
Auto merge of #1547 - daira:1378-witness-cache-assertion-failure.1, r…
…=bitcartel

Add more precise assertions in IncrementNoteWitnesses

Part of #1378
@str4d

This comment has been minimized.

Show comment
Hide comment
@str4d

str4d Oct 17, 2016

Contributor

@ngreatorex @chuckfeast @vaklinov we have added more precise assertions for RC 1 (being released shortly), to help us track down the cause of the bug. Next time your node crashes with this error, please post that crash output here (not the crash output from subsequent starts).

Contributor

str4d commented Oct 17, 2016

@ngreatorex @chuckfeast @vaklinov we have added more precise assertions for RC 1 (being released shortly), to help us track down the cause of the bug. Next time your node crashes with this error, please post that crash output here (not the crash output from subsequent starts).

@str4d str4d modified the milestones: 1.0.0-rc2, 1.0.0-rc1 Oct 17, 2016

@str4d

This comment has been minimized.

Show comment
Hide comment
@str4d

str4d Oct 17, 2016

Contributor

Moving to RC2 where we will hopefully apply a fix.

Contributor

str4d commented Oct 17, 2016

Moving to RC2 where we will hopefully apply a fix.

@str4d

This comment has been minimized.

Show comment
Hide comment
@str4d

str4d Oct 20, 2016

Contributor

Another slightly different data point:

There was a black out and my ubuntu running on virtualbox mining zcash shut down under power loss. when i reboot and try to start up daemon, it gave me this error again zcashd: wallet/wallet.cpp:702: void CWallet::IncrementNoteWitnesses(const CBlockIndex_, const CBlock_, ZCIncrementalMerkleTree&): Assertion `nd->witnesses.size() == 0' failed.

That's the new assertion that when a note is found in a block, its cache must not have any witnesses. So this is a good indication that this is the source of the additional witness.

Contributor

str4d commented Oct 20, 2016

Another slightly different data point:

There was a black out and my ubuntu running on virtualbox mining zcash shut down under power loss. when i reboot and try to start up daemon, it gave me this error again zcashd: wallet/wallet.cpp:702: void CWallet::IncrementNoteWitnesses(const CBlockIndex_, const CBlock_, ZCIncrementalMerkleTree&): Assertion `nd->witnesses.size() == 0' failed.

That's the new assertion that when a note is found in a block, its cache must not have any witnesses. So this is a good indication that this is the source of the additional witness.

@daira

This comment has been minimized.

Show comment
Hide comment
@daira

daira Oct 20, 2016

Contributor

Ok good; now we can focus on that without risk of hunting wild geese.

I don't currently feel like I understand the intended invariants of the witness caching well enough -- which is bad! Does anyone want to pair with me to hunt down this bug/goose/mixed metaphor?

Contributor

daira commented Oct 20, 2016

Ok good; now we can focus on that without risk of hunting wild geese.

I don't currently feel like I understand the intended invariants of the witness caching well enough -- which is bad! Does anyone want to pair with me to hunt down this bug/goose/mixed metaphor?

@str4d

This comment has been minimized.

Show comment
Hide comment
@str4d

str4d Oct 20, 2016

Contributor

@daira if no one else offers, I can pair with you in 1hr10

Contributor

str4d commented Oct 20, 2016

@daira if no one else offers, I can pair with you in 1hr10

@daira

This comment has been minimized.

Show comment
Hide comment
@daira

daira Oct 20, 2016

Contributor

That works well. I added @bitcartel and @ebfull as optional if they want to join.

Contributor

daira commented Oct 20, 2016

That works well. I added @bitcartel and @ebfull as optional if they want to join.

str4d added a commit to str4d/zcash that referenced this issue Oct 20, 2016

@str4d str4d added the has PR label Oct 20, 2016

zkbot pushed a commit that referenced this issue Oct 21, 2016

zkbot
Auto merge of #1580 - str4d:1378-repair-witness-cache, r=daira
Clear witness cache when re-witnessing notes

Closes #1378

zkbot pushed a commit that referenced this issue Oct 21, 2016

zkbot
Auto merge of #1580 - str4d:1378-repair-witness-cache, r=daira
Clear witness cache when re-witnessing notes

Closes #1378

zkbot pushed a commit that referenced this issue Oct 21, 2016

zkbot
Auto merge of #1580 - str4d:1378-repair-witness-cache, r=daira
Clear witness cache when re-witnessing notes

Closes #1378

@zkbot zkbot closed this in #1580 Oct 21, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment