-
Notifications
You must be signed in to change notification settings - Fork 267
refactor(fuel): configuring forc and fuel-core upgrades to the Pyth fuel contract #2858
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
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@@ -374,7 +374,7 @@ fn update_fee(update_data: Vec<Bytes>) -> u64 { | |||
total_fee(total_number_of_updates, storage.single_update_fee) | |||
} | |||
|
|||
#[storage(read, write), payable] | |||
#[storage(read, write)] |
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.
One thing I wanted a second opinion on: one of the new issues that was introduced was that helper functions cannot be listed as "payable" anymore (only functions within the abi are allowed to have that tag). Leaving the tag in there stops the forc build. Removing the tag allows it to build, and all integration tests still pass. I still wanted to make sure that this is a valid removal (i.e. does the payable trait pass down from its calling function).
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.
Yeah i believe it is ok, i think Fuel has borrowed some semantics from EVM (just as it is a layer 2) and in EVM entrypoint functions that are not payable always check that msg.value == 0
// let a: b256 = child_a.into(); | ||
// let b: b256 = child_b.into(); |
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.
remove?
let a: b256 = b256::from_le_bytes(child_a.clone()); | ||
let b: b256 = b256::from_le_bytes(child_b.clone()); |
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'm so confused how little endian could work here. generally raw bytes operations translate to big endian on numbers. also in the docs i see the example is similar.
https://fuellabs.github.io/sway/master/std/primitive.b256.html#method.from_be_bytes
If I'm right. I'm wondering how the tests passed with this. if I'm wrong, i like to know how :?
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 think I misinterpreted the structure of the array, BE is definitely right. For some reason the tests don't change state even when I completely mess with the code. I'll try to rebuild things and see if that changes anything.
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 think you should use from_be_bytes instead. please let me know what you think and i'll look again.
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.
Thanks!
Summary
Swaylend Fuel clients asked us to refactor the fuel contract to work with newer versions of the forc and fuel-core libraries (0.68.9 and 0.43.2 respectively). Simply changing the versions resulted in some compile errors, so code changes had to be made.
Rationale
Client request for support for updated libraries.
How has this been tested?
I refactored the code until forc build worked and the existing integration tests through cargo test all passed.