Create UtxoData and check coinbase maturity

[JoseSK999]

What is the purpose of this pull request?

• Bug fix
• Documentation update
• New feature
• Test
• Other: Refactor

Which crates are being modified?

• floresta-chain
• floresta-cli
• floresta-common
• floresta-compact-filters
• floresta-electrum
• floresta-watch-only
• floresta-wire
• floresta
• florestad
• Other: Benches

Description

I have added the same UtxoData struct from PR #300, but with an additional is_coinbase flag. We get the flag from the utreexo leaf header code, just like the utxo creation height. The utxo creation unix time is not computed nor used anywhere (covered at #300).

The Consensus::verify_transaction now checks the spent coinbase tx outputs maturity. ChainParams has a constant for the maturity period but it's always 100 regardless of the network.

Notes to the reviewers

Most of this diff is refactoring TxOut to UtxoData in functions. I'm generally not using a UtxoMap type alias in order to keep the map types explicit on a first look.

This PR should should make #300 more focused on the timelock checks and unix time computation, and less so on refactors.

[jaoleal]

cACK cc022a0

I'm generally not using a UtxoMap type alias in order to keep the map types explicit on a first look.

The reason to create UtxoMap in #300 is just for better organization and code reading.

This PR should should make #300 more focused on the timelock checks and unix time computation, and less so on refactors.

I`m considerating a total refactor of #300 instead of dealing with conflicts.

[JoseSK999]

The reason to create UtxoMap in #300 is just for better organization and code reading.

Yeah, in this case I would prefer the explicitness of HashMap<OutPoint, UtxoData> unless we have a complex signature (e.g. process_proof)

[jaoleal]

The reason to create UtxoMap in #300 is just for better organization and code reading.

Yeah, in this case I would prefer the explicitness of HashMap<OutPoint, UtxoData> unless we have a complex signature (e.g. process_proof)

Im totally neutral about it because i had to reimplement some funcs just because it was a type alias, so im not sure if its worth the change...

BTW, Tested locally and everything runs fine. have you done some blocks validation to certify this ?

[JoseSK999]

The sync must work since the only consensus change is:

// A coinbase output created at height n can only be spent at height >= n + 100
if utxo.is_coinbase && (height < utxo.creation_height + 100) {
    return Err(tx_err!(txid, CoinbaseNotMatured))?;
}

[jaoleal]

The sync must work since the only consensus change is:

// A coinbase output created at height n can only be spent at height >= n + 100
if utxo.is_coinbase && (height < utxo.creation_height + 100) {
    return Err(tx_err!(txid, CoinbaseNotMatured))?;
}

Checked the code for this rule:

https://github.com/bitcoin/bitcoin/blob/874da961d015620ffcdac91af76b90aaee823926/src/consensus/tx_verify.cpp#L179

It looks the exactly same so:

ACK cc022a0

[Davidson-Souza]

I've successfully completed IBD on this branch. But a nice follow up would be some tests for the negative case (i.e. invalid spents)

ACK cc022a0