Removed this assertion as it was leading to a crash on my testnet4

Ghost commented

2023-10-19 05:52:12 +02:00

(Migrated from bitcoincashcode.org)

It's possible for an output amount to be 0 on a real blockchain -- a miner can mine a txn paying 0 to any address!

Such a thing happened on testnet4.

Here is the testnet4 txn in question that crashed the wallet at startup: 41a258f1abab61bd8c9819666081adca9c5e925a15a5f70b7d3da930a39c7c89 (viewable here: https://tbch4.loping.net/tx/41a258f1abab61bd8c9819666081adca9c5e925a15a5f70b7d3da930a39c7c89).

It pays to a wallet address an amount of "0".

The crash log for it before this change was:

03:36:15 [P2PNet] Peer database loaded: 13 disabled peer count: 0 
    .980 [P2PNet] Starting to load the blockchain  "" 
03:36:16 [P2PNet] Starting to load the blockchain number: "1" 
    . 70 [P2PNet] Blockchain loading completed. Tip: 170589 0000000009c0e6a4bba9f9f5623a6014a29492de9a3803d8b767e31db4c14eba 
    . 76 [10001] Wallet::rebuildBloom() Rebuilding bloom filter. UTXO-size: 0 
    . 76 [10000] FloweePay::init() Found wallet Initial Wallet with segment ID: 40491 
Assertion failed: (output.value > 0), function loadWallet, file Wallet.cpp, line 1933.
Abort trap: 6

I guess the code in question is trying to detect out-of-order tags appearing in the file or something so as to realize when output is not properly initialized.

I wanted to define 0xffffffffffffffffull as the uninitialized output value -- but was unsure if this was safe to do in the code.

This PR is just a suggestion -- perhaps you have a better way to do this, like using std::optional there or something else like a special value that is not 0.

It's possible for an output amount to be 0 on a real blockchain -- a miner can mine a txn paying 0 to any address! Such a thing happened on testnet4. Here is the testnet4 txn in question that crashed the wallet at startup: `41a258f1abab61bd8c9819666081adca9c5e925a15a5f70b7d3da930a39c7c89` (viewable here: https://tbch4.loping.net/tx/41a258f1abab61bd8c9819666081adca9c5e925a15a5f70b7d3da930a39c7c89). It pays to a wallet address an amount of "0". The crash log for it before this change was: ``` 03:36:15 [P2PNet] Peer database loaded: 13 disabled peer count: 0 .980 [P2PNet] Starting to load the blockchain "" 03:36:16 [P2PNet] Starting to load the blockchain number: "1" . 70 [P2PNet] Blockchain loading completed. Tip: 170589 0000000009c0e6a4bba9f9f5623a6014a29492de9a3803d8b767e31db4c14eba . 76 [10001] Wallet::rebuildBloom() Rebuilding bloom filter. UTXO-size: 0 . 76 [10000] FloweePay::init() Found wallet Initial Wallet with segment ID: 40491 Assertion failed: (output.value > 0), function loadWallet, file Wallet.cpp, line 1933. Abort trap: 6 ``` --- I guess the code in question is trying to detect out-of-order tags appearing in the file or something so as to realize when `output` is not properly initialized. I wanted to define `0xffffffffffffffffull` as the uninitialized output value -- but was unsure if this was safe to do in the code. This PR is just a suggestion -- perhaps you have a better way to do this, like using `std::optional` there or something else like a special value that is not 0.

Ghost commented

2023-10-19 08:59:06 +02:00

(Migrated from bitcoincashcode.org)

I love asserts, you'll find a boatload all over the code. And indeed removing one or two that are not doing their job is Ok. No biggy. Its mostly useful for development on those features, but they are quite stable now.

So, yeah, I'll merge it as is. Thanks!

For the record, release builds do not have asserts compiled in.

I love asserts, you'll find a boatload all over the code. And indeed removing one or two that are not doing their job is Ok. No biggy. Its mostly useful for development on those features, but they are quite stable now. So, yeah, I'll merge it as is. Thanks! For the record, release builds do not have asserts compiled in.

Removed this assertion as it was leading to a crash on my testnet4 #23