Account-base fee collection

[xgreenx]

Closes #1269

This PR also adapts fuel-core to use fuel-vm 0.39.

• The recipient is a ContractId instead of Address. The block producer should deploy its contract to receive the transaction fee. The collected fee is zero until the recipient contract is set.
• The Mint transaction is reworked with new fields to support the account-base model. It affects serialization and deserialization of the transaction and also affects GraphQL schema.
• I limited the number of selected transactions from the TxPool to u16::MAX to avoid overflow.
• The Mint transaction is the last transaction in the block instead of the first.
• Applied changes for BlockHeight to use succ and pred functions.

[bvrooman]

Why do we have input_contracts as well as input_contract?

From reading the code, it looks like input_contracts is used for Script and Create transactions (or anything that is not a Mint transaction), while input_contract is used for Mint transactions.

Maybe it would be helpful to change the names to better describe their usage and/or add some comments.

[bvrooman]

Any update here?

[xgreenx]

I've added the comments into the code itself=)

[bvrooman]

Maybe succ() should be infallible?

[xgreenx]

After 132 years, we can overflow, so it is better to panic=D

[bvrooman]

Yes, we can panic, but the panic could be inside succ. Right now, we always use this pattern:

Some(Self(self.0.checked_add(1)?)).expect("expect valid")

where we always add expect (or unwrap) - a quick search of the codebase shows we never check the Option.

We could just put the expect in succ and achieve the same thing, but simpler.

But it's not a big deal - just an observation.

[bvrooman]

Just one remaining question about adding comments about the input_contracts versus input_contract and output_contracts versus input_contract. It's up to you if you would like to add comments or change anything there.

[Voxelot]

Should the tx_count include the mint tx? It may be confusing later on down the road if the count gets used for something else and isn't accurate.

[xgreenx]

The execute_transaction closure increases the counter at the end and includes the transaction=)

[MitchTurner]

What is the "helper function"? This is a field on a struct.

[MitchTurner]

Gotcha. The function isn't public though, but this doc comment is public, so maybe mentioning the function isn't necessary as long as you say it's generated rather than part of the original Transaction.

[MitchTurner]

I think it would improve readability a lot to break down these functions into sub-functions. These functions are long. The names of the sub-functions can help a lot as well.

[MitchTurner]

This is an improvement. I don't know if I'm a fan of the function being in shared scope, it is about as cluttered as before. Also, having the "else" case be encapsulated as a function wouldn't hurt either, but I don't want to create too much extra work for you :). Clean Code suggests avoiding functions that are more than 20 lines long, that is an arbitrary number, but I tend to agree in spirit--breaking down functions makes a world of difference on cognitive load for a reader.

[xgreenx]

I don't want to add one more function that accepts 8 arguments=)

[MitchTurner]

Okay. I'll leave it as a nit, however I think we shouldn't let something like that keep us from making our code more friendly and sympathetic to the next engineer.