refactor: move TxPoolError from fuel-core-types to fuel-core-txpool

[netrome]

Linked Issues/PRs

• Closes Move TxPoolError from fuel-core-types to fuel-core-txpool #2082

Description

This PR does three things:

1. The Error type defined in crates/types/src/services/txpool.rs is moved to crates/services/txpool/src/error.rs.
2. This error type now also implements From<WasmValidityError>.
3. Refactor to use derive_more instead of thiserror for the error type.

Checklist

• Breaking changes are clearly marked as such in the PR description and changelog
• New behavior is reflected in tests
• The specification matches the implemented behavior (link update PR if changes are needed)

Before requesting review

• I have reviewed the code myself
• I have created follow-up issues caused by this PR and linked them here

[netrome]

Will rebase on master and mark the PR as ready once #2080 is merged.

[rymnc]

I understand most of this code has been copied from a previous PR, so I apologize for the review comments changing some semantics of the display messages :)

[rymnc]

Suggested change

	fmt = "Transaction is not inserted. More priced tx has created contract with ContractId {_0:#x}"
	fmt = "Transaction is not inserted. Higher priced tx has created contract with ContractId {_0:#x}"

[rymnc]

perhaps we also want to add the tx hash for the already spent utxo like the above error :)

[netrome]

From what I can see, we only have ContractId here so I believe you mistook this for the previous NotInsertedCollision variant.

[netrome]

But I've updated the message ✅

[rymnc]

yup, I meant to ask if we should include the higher priced tx hash like we have done in other error cases, what do you think?

[netrome]

Oh sorry I misunderstood you. Yeah that sounds helpful. I'll see if I can include it.

[netrome]

Getting the txid here doesn't look trivial based on my limited understanding of the code unfortunately.

Reasoning:
This variant is only created in crates/services/txpool/src/containers/dependency.rs in a loop that looks like this:

        for output in tx.outputs() {
            if let Output::ContractCreated { contract_id, .. } = output {
                if let Some(contract) = self.contracts.get(contract_id) {
                    // we have a collision :(
                    if contract.is_in_database() {
                        return Err(Error::NotInsertedContractIdAlreadyTaken(*contract_id))
                    }
                    // check who is priced more
                    if contract.tip > tx.tip() {
                        // new tx is priced less then current tx
                        return Err(Error::NotInsertedCollisionContractId(*contract_id))
                    }
                    // if we are prices more, mark current contract origin for removal.
                    let origin = contract.origin.expect(
                        "Only contract without origin are the ones that are inside DB. And we check depth for that, so we are okay to just unwrap"
                        );
                    collided.push(*origin.tx_id());
                }
            }
            // collision of other outputs is not possible.
        }

The contract variable here is of the following type

#[derive(Debug, Clone)]
pub struct ContractState {
    /// is Utxo spend as other Tx input
    used_by: HashSet<TxId>,
    /// how deep are we inside UTXO dependency
    depth: usize,
    /// origin is needed for child to parent rel, in case when contract is in dependency this is how we make a chain.
    origin: Option<UtxoId>,
    /// gas_price. We can probably derive this from Tx
    tip: Word,
}

which does not seem to hold the information about which txid created the contract.

There may be other ways to fetch the txid, but that would complect the code for what seems to be a very unusual error scenario so I'll leave this as-is for now if that's alright with you.

[rymnc]

not a review comment, but rather a question
is this for regulatory purposes? @xgreenx

[xgreenx]

No, it is for the case of bugs in the network that causes money printing=)

[netrome]

I understand most of this code has been copied from a previous PR, so I apologize for the review comments changing some semantics of the display messages :)

No worries, and thanks for the suggestions. This is a good opportunity to improve the messages. I'll update them.

[rymnc]

LGTM, but won't approve (in case it gets automerged) because someone with more experience with the codebase should probably sign off on it :)

[xgreenx]

LGTM=)