In some function missed MessageSigned and MessagePredicate functionality
#186
Conversation
src/transaction/types/input.rs
Outdated
| @@ -234,6 +235,8 @@ impl Input { | |||
| } | |||
| } | |||
|
|
|||
| // TODO: Maybe we need to move `ResourceId` from `fuel-core` into `fuel-tx`(or `fuel-type`) and | |||
There was a problem hiding this comment.
If not, I will remove this TODO, if yes, I will create an issue=)
There was a problem hiding this comment.
That depends on whether or not the resource id is going to be an attribute of the transaction. If yes, then it should be moved here
xgreenx
requested review from
vlopes11,
leviathanbeak and
Voxelot
and removed request for
vlopes11
src/consts.rs
Outdated
| use fuel_types::{Bytes32, Salt}; | ||
| use fuel_types::{AssetId, Bytes32, Salt}; | ||
|
|
||
| pub const BASE_ASSET: AssetId = AssetId::zeroed(); |
There was a problem hiding this comment.
If this is becoming a public constant, then it should be added as associated constant of AssetId. ie AssetId::BASE_ASSET (or equivalent name)
There was a problem hiding this comment.
src/transaction/types/input.rs
Outdated
| @@ -234,6 +235,8 @@ impl Input { | |||
| } | |||
| } | |||
|
|
|||
| // TODO: Maybe we need to move `ResourceId` from `fuel-core` into `fuel-tx`(or `fuel-type`) and | |||
There was a problem hiding this comment.
That depends on whether or not the resource id is going to be an attribute of the transaction. If yes, then it should be moved here
src/transaction/types/input/repr.rs
Outdated
| @@ -25,7 +25,7 @@ impl InputRepr { | |||
| pub const fn owner_offset(&self) -> Option<usize> { | |||
| match self { | |||
| Self::Coin => Some(INPUT_COIN_OWNER_OFFSET), | |||
| Self::Message => None, | |||
| Self::Message => Some(INPUT_MESSAGE_RECIPIENT_OFFSET), | |||
There was a problem hiding this comment.
This isn't really an owner but a recipient. I don't recommend we mix things because we create a magic box effect (that we don't know what exactly is happening internally in the function, provided only its name)
src/transaction/types/input.rs
Outdated
| @@ -247,9 +250,10 @@ impl Input { | |||
| pub const fn input_owner(&self) -> Option<&Address> { | |||
| match self { | |||
| Self::CoinSigned { owner, .. } | Self::CoinPredicate { owner, .. } => Some(owner), | |||
| Self::MessageSigned { .. } | Self::MessagePredicate { .. } | Self::Contract { .. } => { | |||
| None | |||
| Self::MessageSigned { recipient, .. } | Self::MessagePredicate { recipient, .. } => { | |||
There was a problem hiding this comment.
Same thing as Victor mentioned below, we shouldn't mix recipient and owner behind the same getter. It's annoying since they are basically the same thing, but it makes the public interface confusing if we have inconsistencies like this.
In the tests for FuelLabs/fuel-core#632, I found the handling of the
MessageSignedandMessagePredicateinputs is missing in some methods.