Coinbase maturity part 2

[ghost]

Problem found by @beggsdl

beggsdl: I did this testing by solo mining to my core wallet. Here is a screenshot of the patched v8 wallet confirming a coinbase TX:

beggsdl: Here is that same coinbase TX in the 7.17.3 wallet:

beggsdl: The transaction detail screen in v8 says it needs to mature for 101 blocks before being spent, but you can see in the Status and Credit, that it is only confirming for 9.

beggsdl: This then affects the Immature and Available balances in the v8 wallet (top) compared to the v7 wallet (bottom):

beggsdl: I was then able to try to prematurely spend (before 100 confirmations) some of the coinbase TX from the v8 wallet (the v7 wallet did not allow me to do this due to insufficient funds):

beggsdl: This 50 DGB transaction is now stuck in mempool of the v8 wallet, even 17 hours after being created.

---

@JaredTate "It looks like there might be a couple of other places where COINBASE_MATURITY_2 needs added properly to fix the issue as @beggsdl mentioned above."

txmempool.cpp
wallet.cpp

#157 (comment)

---

With this pr. Generated coins must mature 101 blocks before they can be spent.

[saltedlolly]

@beggsdl thanks for identifying the issue and @Jongjan88 thanks for preparing this PR. :)

[beggsdl]

Thanks @Jongjan88 for creating this. I think it is important for this PR to get merged along with PR 157 before RC3.

[saltedlolly]

Both your contributions are greatly appreciated!

[saltedlolly]

Were there any other places where COINBASE_MATURITY needed fixing?

[beggsdl]

Were there any other places where COINBASE_MATURITY needed fixing?

I have a long list of files related to tests that I feel need to be addressed. I need to finish going through another set of files to finalize my list. I can add that as a separate Issue once I am done, hopefully in the next day or so. What's confusing is that the tests use COINBASE_MATURITY (8) and COINBASE_MATURITY_ORIGINAL (100).

Other than the list I am creating, I think COINBASE_MATURITY is addressed everywhere.

[saltedlolly]

This PR addresses #161

[ycagel]

cACK. Great work. I would want the blessings of @gto90 and @JaredTate before any merging.

[JaredTate]

ACK. This PR looks great! Code compiles & client runs. Thanks for doing this @Jongjan88 Awesome work!

[beggsdl]

Can someone check this code to make sure it does not need to be addressed also? Need to make sure this doesn't need to be changed too.

digibyte/src/bench/block_assemble.cpp

Lines 26 to 36 in e8247b0

	// Collect some loose transactions that spend the coinbases of our mined blocks
	constexpr size_t NUM_BLOCKS{130};
	std::array<CTransactionRef, NUM_BLOCKS - COINBASE_MATURITY + 1> txs;
	for (size_t b{0}; b < NUM_BLOCKS; ++b) {
	CMutableTransaction tx;
	tx.vin.push_back(MineBlock(test_setup->m_node, P2WSH_OP_TRUE));
	tx.vin.back().scriptWitness = witness;
	tx.vout.emplace_back(1337, P2WSH_OP_TRUE);
	if (NUM_BLOCKS - b >= COINBASE_MATURITY)
	txs.at(b) = MakeTransactionRef(tx);
	}

[JaredTate]

Can someone check this code to make sure it does not need to be addressed also? Need to make sure this doesn't need to be changed too.

digibyte/src/bench/block_assemble.cpp

Lines 26 to 36 in e8247b0

	// Collect some loose transactions that spend the coinbases of our mined blocks
	constexpr size_t NUM_BLOCKS{130};
	std::array<CTransactionRef, NUM_BLOCKS - COINBASE_MATURITY + 1> txs;
	for (size_t b{0}; b < NUM_BLOCKS; ++b) {
	CMutableTransaction tx;
	tx.vin.push_back(MineBlock(test_setup->m_node, P2WSH_OP_TRUE));
	tx.vin.back().scriptWitness = witness;
	tx.vout.emplace_back(1337, P2WSH_OP_TRUE);
	if (NUM_BLOCKS - b >= COINBASE_MATURITY)
	txs.at(b) = MakeTransactionRef(tx);
	}

We can change the 130 to 200. Great find, although we can have a discussion on what is the appropriate # here. 200 is the default for BTC. This is all in the bench folder, which is for benchmarking. Benchmarking is something I have never used/ done and it is not complete for the entire protocol. DGB codebase has never been even configured for benchmarking in the code.

This benchmarking code can help us performance test parts of code and have performance data, but it's nothing essential for core protocol and consensus.

More benchmarking info here:https://github.com/DigiByte-Core/digibyte/blob/develop/doc/benchmarking.md

Benchmarking

DigiByte Core has an internal benchmarking framework, with benchmarks
for cryptographic algorithms (e.g. SHA1, SHA256, SHA512, RIPEMD160, Poly1305, ChaCha20), rolling bloom filter, coins selection,
thread queue, wallet balance.

Running

For benchmarking, you only need to compile digibyte_bench. The bench runner
warns if you configure with --enable-debug, but consider if building without
it will impact the benchmark(s) you are interested in by unlatching log printers
and lock analysis.

make -C src digibyte_bench

After compiling digibyte-core, the benchmarks can be run with:

src/bench/bench_digibyte

[beggsdl]

Thanks for the clarification on that @JaredTate.

[gto90]

With one minor formatting fix I think I am prepared to give this a cACK.

[JaredTate]

ACK. Thank you to everyone for your help on this! Will be great to get this tested on testnet. This code compiles & runs fine for me.

[ycagel]

cACK. Good find @gto90! Thanks for everyone's efforts.

[gto90]

cACK

Thank you all for your generous contributions!