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.

Sign up for GitHub

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

Jump to bottom

feat: typegen support for passing Uint8Array as Vec<u8> #2884

Closed
wants to merge 2 commits into from
Closed

feat: typegen support for passing Uint8Array as Vec<u8> #2884

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
10ec034
feat: typegen support for passing `Uint8Array` as `Vec<u8>`
Dhaiwat10 Aug 5, 2024
4d2ea1d
Merge branch 'master' into dp/uintarray-vec-typegen
Dhaiwat10 Aug 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/cuddly-carrots-compare.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@fuel-ts/abi-typegen": patch
---

feat: add typegen support for passing `Uint8Array` as `Vec<u8>`
8 changes: 5 additions & 3 deletions packages/abi-typegen/src/templates/common/common.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
{{header}}

import type { BigNumberish } from 'fuels';

/**
* Mimics Sway Enum.
* Requires one and only one Key-Value pair and raises error if more are provided.
Expand All @@ -14,10 +16,10 @@ export type Enum<T> = {
*/
export type Option<T> = T | undefined;

export type Vec<T> = T[];
export type Vec<T> = T extends BigNumberish ? T[] | Uint8Array : T[];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will be a problem for u16 and u32 values because BigNumberish is used for them as well, but the Uint8Array cannot store their values and that is the reason why I created #2785. Users might get the false impression that they can pass Uint8Array for u16 and u32 vectors.

I think that this problem cannot be solved on the level of the generated types like this; rather, the checks must be done somewhere in typegen where abi type parsing is happening.


/**
/**
* Mimics Sway Result enum type.
* Ok represents the success case, while Err represents the error case.
*/
*/
export type Result<T, E> = Enum<{Ok: T, Err: E}>;
13 changes: 13 additions & 0 deletions packages/fuel-gauge/src/vectors.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,19 @@ describe('Vector Tests', () => {
expect(value).toStrictEqual(INPUT);
});

it('should test u8 vector input/output as Uint8Array', async () => {
using contractInstance = await setupContract();
const INPUT = [8, 6, 7, 5, 3, 0, 9];
const INPUT_AS_UINT8_ARRAY = Uint8Array.from(INPUT);
Comment on lines +39 to +40
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const INPUT = [8, 6, 7, 5, 3, 0, 9];
const INPUT_AS_UINT8_ARRAY = Uint8Array.from(INPUT);
const INPUT: Vec<BigNumberish> = [8, 6, 7, 5, 3, 0, 9];
const INPUT_AS_UINT8_ARRAY: Vec<BigNumberish> = Uint8Array.from(INPUT);

Is it worth adding the types here, to ensure they work as intended?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@petertonysmith94 there's a separate issue for those types #2785, but I'm gonna be making changes to this PR regardless: #2884 (comment)


const { waitForResult } = await contractInstance.functions
.echo_u8(INPUT_AS_UINT8_ARRAY)
.call<Uint8Array>();
const { value } = await waitForResult();
Copy link
Contributor

@nedsalk nedsalk Aug 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are always returning a number[] here in runtime so this should never be Uint8Array. As mentioned above, this will have to be handled differently because the Vec type is shared between all vectors, both input and output ones.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nedsalk that's a great catch. I completely missed this.

I considered this deeper approach with changes deeper inside the Vec type but thought I might be able to get away with something quick like this. I will revert this PR and come back with a better approach that is more modular and flexible


expect(Uint8Array.from(value)).toStrictEqual(INPUT_AS_UINT8_ARRAY);
});

it('should test u16 vector input/output', async () => {
using contractInstance = await setupContract();
const INPUT = [8, 6, 7, 5, 3, 0, 9];
Expand Down
Loading