feat: use SDK type for tx in TransactionResponse

[MujkicA]

Closes #959

I've explored several approaches to make our API consistent here.

One option was to have TransactionResponse hold a `Box, but because of the object safety requirements, this had the following downsides:

• No Into<FuelTransaction> super trait possible, this would need to become a separate method
• TransactionResponse would no longer be Clone
• Box needs to be cloned internally to keep with_ methods + a Box would have to be returned which could cause confusion
• And the worst one, a user would need to downcast the tx from a TransactionResponse to either Script or Create

The second option was to use an enum for the implementors of Transaction and then have the enum implement it too. The downside here is that we get a lot of repeated match statements to delegate every method of Transaction to the corresponding variant. Macros didn't provide a significant improvement here either.

Ultimately I decided to keep the enum but not implement Transaction for it. This means that we don't change our API anywhere else but also that the user has to potentially match the enum every time to use the underlying Transaction methods.

Checklist

• I have linked to any relevant issues.
• I have added necessary labels.
• I have done my best to ensure that my PR adheres to the Fuel Labs Code Review Standards.
• I have requested a review from the relevant team or maintainers.

[hal3e]

LGTM

[iqdecay]

lgtm

[segfault-magnet]

Can't make up my mind. Hate that the user has to always match before doing anything useful with the type.

But then again I can imagine how ugly the delegation might have looked.

Besides the impact on maintenance cost, is there any other reason for not implementing the delegating methods so that it may be used as is?

[iqdecay]

Can't make up my mind. Hate that the user has to always match before doing anything useful with the type.

Good catch, didn't realise myself. I'd like to know as well before merging.

[hal3e]

@FuelLabs/sdk-rust did we decide what to do with this PR ?

[MujkicA]

Can't make up my mind. Hate that the user has to always match before doing anything useful with the type.

But then again I can imagine how ugly the delegation might have looked.

Besides the impact on maintenance cost, is there any other reason for not implementing the delegating methods so that it may be used as is?

It was only about the maintenance. IMO matching isn't that bad considering that this type is only used when fetching a TransactionResponse. I imagine that's not that common of a use case.

The last commit has the changes with delegation.

[segfault-magnet]

Might be confusing down the line to have Transaction trait and TransactionType type

But we'll solve it if it becomes an issue.

[Salka1988]

👍

[hal3e]

LGTM