-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
feat(abi): Update interface to handle array and enum contract arguments #419
Conversation
fn take_array_string_return_single_element(a: [str[3];3]) -> str[3] { | ||
a[0] | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is still erroring out; The return receipt is type Panic
. Anyone know what is going on?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc: @mohammadfawaz
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm interesting.. Any idea whether this is an issue with the compiler or fuels-ts
? Does fuels-rs
have the same problem?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wrote some test for fuels-rs
here @digorithm can you check these look good?
Generating the abi with cargo run --bin build-test-projects
works, but on cargo test
I get this error
error: proc macro panicked
--> packages/fuels/tests/harness.rs:2310:5
|
2310 | / abigen!(
2311 | | TestContract,
2312 | | "packages/fuels/tests/test_projects/array_contract_inputs/out/debug/array_contract_inputs-abi.json"
2313 | | );
| |_____^
|
= help: message: called `Result::unwrap()` on an `Err` value: InvalidType("Expected parameter type `str[n]`, found `[str[3]; 3]`")
I'm on forc 0.19.0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @mohammadfawaz ^^
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Created FuelLabs/sway#2410 FuelLabs/fuels-rs#505 as followups. I revisit this and not block the PR
53e649c
to
173ee6c
Compare
Total Coverage: 89.11% Coverage Report
|
Total Coverage: 89.12% Coverage Report
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small request for test coverage
Total Coverage: 89.12% Coverage Report
|
@QuinnLee I think it's nice to have both your original test and this new test. |
Total Coverage: 89.12% Coverage Report
|
🤦🏼 my bad. I think I still have brain fog from covid. I pushed an update - more tests with bool, b256 and strings |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Amazing!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Update the function fragment to match specs and return the correct format for
enums
andarrays
. Some refactors were done to make sure it works with arrays with different types.There is still an underlying issue with one of the functions; returning a single element from an array is raising an error; see below.
Closes #420