Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TOB-FUEL-13: Specification about ABI encoding is outdated #1107

Closed
xgreenx opened this issue Aug 28, 2023 · 3 comments
Closed

TOB-FUEL-13: Specification about ABI encoding is outdated #1107

xgreenx opened this issue Aug 28, 2023 · 3 comments
Labels
audit-report Related to the audit report

Comments

@xgreenx
Copy link
Contributor

xgreenx commented Aug 28, 2023

Description

The ABI encoding specification is missing the variable-length array type. The implementation also supports the Vector type as shown in the following figure. Though, the Vector type is not documented in the specification. The same is true for tuples.

Figure 13.1: Enumeration of supported Token types. (fuels-rs/packages/fuels-core/src/types.rs#71–94)

71    #[derive(Debug, Clone, PartialEq, EnumString)]
 72    #[strum(ascii_case_insensitive)]
 73    pub enum Token {
 74       // Used for unit type variants in Enum. An "empty" enum is not represented
as Enum<empty box>,
 75       // because this way we can have both unit and non-unit type variants.
 76       Unit,
 77       U8(u8),
 78       U16(u16),
 79       U32(u32),
 80       U64(u64),
 81       U128(u128),
 82       U256(U256),
 83       Bool(bool),
 84       B256([u8; 32]),
 85       Array(Vec<Token>),
 86       Vector(Vec<Token>),
 87       String(StringToken),
 88       Struct(Vec<Token>),
 89       #[strum(disabled)]
 90       Enum(Box<EnumSelector>),
 91       Tuple(Vec<Token>),
 92       RawSlice(Vec<u64>),
 93       Bytes(Vec<u8>),
 94    }

Recommendations

Short term, extend the specification and add a specification for vectors and tuples.
Long term, implement a definition of ready (DoR) for issues, which requires code reviewers to verify that the specification matches the code.

@xgreenx
Copy link
Contributor Author

xgreenx commented Aug 29, 2023

Related specification issue FuelLabs/fuel-specs#505

@digorithm
Copy link
Member

Closing this because it seems to be resolved by #1066.

@iqdecay
Copy link
Contributor

iqdecay commented Oct 4, 2023

Just to clarify: the issue is with the fuels-specs repo, not fuels-rs. #1066 didn't fix this issue, simply clarified how coding/decoding happened, but not how those types are coded/decoded, especially not Vectors. Currently working on improving the specs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
audit-report Related to the audit report
Projects
None yet
Development

No branches or pull requests

3 participants