Transaction Consensus Rules

[jaoleal]

See #162 for context.
After a reading at crates/floresta-chain/, based on commenting and codebase...
Floresta, what concern the validation of Transactions, has

• In block transaction structure validation. ( Verifies if has, at least, one transaction per block, if has coinbase, etc...)
• Script validation.
• PrevOut validation. (If has a valid PrevOut).

So, based on Core validation.cpp and @Davidson-Souza comment on #162

The lacking Rules are:

Values validation.

- Output value is equal or less than total input (Implemented, just needs tests)

- none value is negative

- Any value is not greater than the number in satoshis of the maximum possible bitcoins. (MAX_MONEY = 21000000 * 100 000 000)

- Double Input Spend. (The case that has more than one equal input, mostly referring to the same utxo)

- Inputs are spending UTXO.

Size validation

- Size, without witness, in vbytes has to be less than MAX_BLOCK_WEIGHT(?)

• no more than 80000 sigops
  What is a sigop
  Their values
• ScriptPubKey Size has to be >2 and <520 bytes
• if coinbase, scriptSig has to be >2 and <100 bytes
• Absolute and Relative Locktimes defined by sequence.

This list can and will change, lacking rules that got implemented will change to Rule
Draft will be ready after all Rules are properly tested.

So, for block validation we do have three points of validation
partial_chain.rs with PartialChainStateInner::process_block() function that delegates the block validation itself for PartialChainStateInner::validate_block(), which verifies:

• merkle_root
• witness_commitment
• prev_block
• transactions (with the validations that was described earlier)

The second point being the ChainState<PersistedState>::validate_block() that has the same functionality as the other validate_block() function just described above, and BlockchainInterface trait that has a unimplemented validate_block() function.

Since we have a Chainstate<PersistedState>::validate_header() we can consider that the known rules are implemented but not tested.

[jaoleal]

I think thats it for transactions

[Davidson-Souza]

I think you need to impose some constraints on the scriptSig sizes too. IIRC only taproot can have unlimited script sizes

[jaoleal]

I`m including the nixify branch so i can already work with flakes

[jaoleal]

@Davidson-Souza what do you think ?

[jaoleal]

After refactoring to functions, the code improve and we can now follow to tests on transactions validations.

[jaoleal]

Yeah, but the design of the code will not help in this case. Itll only fail, we will not know why and what triggered that. Not enough info, not good.

Done some unitary that cover the errors that the new functions has to output.
This should be it for transactions.
For sake of organization Ill rename the draft so we can merge it well.

A personally talk with @Davidson-Souza we established that the objective for block validation code is just some rewriting and implementation to the rest of the code. The first comment explains the actual state of block validation in Florestad.

So doing it in a separate PR right after this one merged.

Tagging @Davidson-Souza for a final review.

[jaoleal]

Strange... Clippy is asking things out of the scope of this PR
maybe we consider a Issue to fix this

[Davidson-Souza]

Strange... Clippy is asking things out of the scope of this PR

The ones I'm seeing are due of an assert_eq! with unit type. Replace it with a assert!(something().is_ok())

[Davidson-Souza]

Also, could you please rebase and squash this?

[jaoleal]

Done, Squashed and lint satisfied

[jaoleal]

Just two things that I need to expose.

About the bip34 rust-bitcoin implementation.
It automatically validates while calling is_coinbase().
checks bip34, the txid and retrieve the actual boolean about it.
so, no need to reimplement that.

And the constraint of having only 1 coinbase.
we do not deal with the case where we do not have any coinbase because itll automatically fails when calculating the subsidy.
And will fail when have more because of the simplest n == 0 before entering the check. An extra coinbase will fail when treated as a regular transaction.

[jaoleal]

Removed Sequence and time validation so we can implement it on #187 since we need some refactoring

[Davidson-Souza]

I've tried to run on the latest commit on mainnet, and I got an error:

[2024-07-09 14:23:46 ERROR floresta_wire::p2p_wire::sync_node] Invalid block Header { block_hash: 0x00000000000000000001711fd54f6864cb693f1f5f87f6a9007bf7a6c5dd30f7, version: Version(642318336), prev_blockhash: 0x00000000000000000002447c802760582b80022f937d754aece888ec9e96f212, merkle_root: 0x7a2b90440f925f9e32f6c6ddda47206adc0b29f7bb8fc5295de0ef95ab7d6bf5, time: 1719078530, bits: CompactTarget(386096421), nonce: 2054740483 } received by peer 0 reason: BlockValidation(InvalidTx("Some input's Scriptsig has more than 520 bytes on"))

But this block is part of the main chain

[jaoleal]

Hmmm, that's interesting... i need to make better error logs

[jaoleal]

Did not change anything of features or rules, but now we can know which transaction we are invalidating