Stabilize encoding v1 #5727
Assignees
Labels
compiler
General compiler. Should eventually become more specific as the issue is triaged
Comments
IGI-111
added
the
compiler
This was referenced Apr 11, 2024
feat!: deprecate multiple encoding version support for configurable constants
FuelLabs/fuels-ts#2040
Merged
This was referenced Apr 17, 2024
hal3e
added a commit
that referenced
this issue
Apr 19, 2024
) - Added a version of `call_with_function_selector` compatible with the new encoding. - Added a version of `create_payload` compatible with the new encoding. - Removed `contract_id_to_bytes`. - Removed `call_with_function_selector_vec` and corresponding tests. - Rename `low_level_call_bytes` test project to `low_level_call`. relevant issue: #5727 BREAKING CHANGE: Removed `call_with_function_selector_vec`
8 tasks
IGI-111
pushed a commit
that referenced
this issue
Apr 25, 2024
## Description This PR sets the "new encoding" (from now on will be called "encoding v1") as the default. We can still disable it using `no_encoding_v1`, which switches back to "encoding v0". Actions that needs to be done after this being merged will exist in #5727 New features - ABI Super traits; - AbiEncode buffer dynamic sizing; Bugs Fixed - `ContractCall` intrinsic interaction effect was not set correctly; Fixing warnings and error messages - Better error message when core-lib is not available for scripts/contracts/predicates; - Better error message when main inputs/outputs are unknown or error types; - Better error message when main inputs/outputs do not implement AbiEncode/AbiDecode; - Don't warn impurity attributes on the "__entry" fn; - Don't warn CEI on the "__entry" fn. Our CEI analysis, currently, does not recognize `Never`. This means it does not realize it is impossible to call two contract functions; Test Disabled (needs to be enabled again in the future) - should_pass/language/name_resolution_after_monomorphization - should_pass/language/shadowing/shadowed_glob_imports - should_pass/language/name_resolution_inside_intrinsics - sdk-harness/external_proxy test is not working. I am assuming it is the LDC bug that is already fixed on version 0.25. What is happening is that the literal "double_value" has the correct length, but some random data. Which makes the method selector fails. Only after we call LDC. The proxy contract is working. Test generating more warnings than before - should_pass/dca/contract/superabi_contract_calls What happens here is that when we implement a trait for `Contract`, we actually generate two functions: one prefixed `__contract_entry` that is called by the method selector; and another one normal, that can be called freely. So, if the trait method is never called manually, it is marked as dead. - should_pass/dca/contract/abi_fn_params I actually think the new warning is correct and nothing here needs to be done. Test with fewer warnings than before - should_pass/dca/unused_fields auto-impl is making all fields being used. so no dead code warnings are being generated. We need to fix this. Changes to std-lib - Functions that return data about call context changed the semantic. `first_param` and `second_param` return the value as the VM sees them. We now have `called_method` and `called_args`. This means that we can change the protocol later and still keep these four functions always working and with meaningful names. - - predicate_data also was updated to use encoding v1 protocol. ICE - increase_buffer_if_needed implementation is a little bit strange because does not work as a method inside `Buffer`. For some reason, it is generating an ICE. I need to create an issue so we can fix, and improve the implementation here. - `Buffer` used by AbiEncode needs a `push_bytes` so we can be more efficient when encoding Bytes and others. ## Checklist - [ ] I have linked to any relevant issues. - [ ] I have commented my code, particularly in hard-to-understand areas. - [ ] I have updated the documentation where relevant (API docs, the reference, and the Sway book). - [ ] If my change requires substantial documentation changes, I have [requested support from the DevRel team](https://github.com/FuelLabs/devrel-requests/issues/new/choose) - [ ] I have added tests that prove my fix is effective or that my feature works. - [ ] I have added (or requested a maintainer to add) the necessary `Breaking*` or `New Feature` labels where relevant. - [ ] I have done my best to ensure that my PR adheres to [the Fuel Labs Code Review Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md). - [ ] I have requested a review from the relevant team or maintainers.
xunilrj
added a commit
that referenced
this issue
May 12, 2024
## Description
This PR uses `AbiEncode` on configurables. We can imagine that
```sway
configurable {
SOMETHING: u64 = 1
}
fn main() -> u64 {
SOMETHING
}
```
will be desugared into
```sway
configurable {
SOMETHING: slice = encode(1)
}
fn main() -> u64 {
abi_decode(SOMETHING)
}
```
To allow this, now the whole `encode` function and all trait impls run
inside `const_eval`. To make this work, three new intrinsic were
implemented: `EncodeBufferEmpty`, `EncodeBufferAppend`, and
`EncodeBufferAsRawSlice`.
`EncodeBufferEmpty` creates an empty "encoding buffer", which is
composed of a pointer to the buffer, the buffer capacity, and how many
bytes were written already.
`EncodeBufferAppend` appends any primitive data types to the buffer.
This intrinsic does not mutate its argument, it returns a new "encoding
buffer". If no reallocation is needed, the pointer and the capacity stay
the same.
`EncodeBufferAsRawSlice` returns a slice with is composed of the buffer
pointer and its length (not the capacity).
### Errors
Some constant expressions cannot live inside the data section because
their encoding depends on some instance value, for example: string
slices, Vecs, Sttrings, Bytes etc... For these we now we return an error
in these cases, like
```
error
--> /home/xunilrj/github/sway/test/src/e2e_vm_tests/test_programs/should_pass/language/configurable_consts/src/main.sw:33:5
|
31 |
32 | C6: str[4] = __to_str_array("fuel"),
33 | C6_2: str = "fuel as str",
| ^^^^ This code cannot be evaluated to a configurable because its size is not always limited.
34 | C7: [u64; 4] = [1, 2, 3, 4],
35 |
|
____
```
## Future TODOs
This PR adds two more tasks to
#5727:
- Enable sdk-harness tests that were ignore. SDK needs to update
configurable encoding.
- Correctly error when a type with custom impl for AbiEncode is used in
configurables
- avoid decoding on each use
## Checklist
- [x] I have linked to any relevant issues.
- [x] I have commented my code, particularly in hard-to-understand
areas.
- [x] I have updated the documentation where relevant (API docs, the
reference, and the Sway book).
- [x] If my change requires substantial documentation changes, I have
[requested support from the DevRel
team](https://github.com/FuelLabs/devrel-requests/issues/new/choose)
- [x] I have added tests that prove my fix is effective or that my
feature works.
- [x] I have added (or requested a maintainer to add) the necessary
`Breaking*` or `New Feature` labels where relevant.
- [x] I have done my best to ensure that my PR adheres to [the Fuel Labs
Code Review
Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md).
- [x] I have requested a review from the relevant team or maintainers.
---------
Co-authored-by: Igor Rončević <ironcev@hotmail.com>
8 tasks
xunilrj
added a commit
that referenced
this issue
May 13, 2024
## Description This PR is part of #5727 and it is just doing the minimal changes to update `fuels-rs` to version 0.61, given we don´t need to use `master` anymore. ## Checklist - [x] I have linked to any relevant issues. - [x] I have commented my code, particularly in hard-to-understand areas. - [x] I have updated the documentation where relevant (API docs, the reference, and the Sway book). - [x] If my change requires substantial documentation changes, I have [requested support from the DevRel team](https://github.com/FuelLabs/devrel-requests/issues/new/choose) - [x] I have added tests that prove my fix is effective or that my feature works. - [x] I have added (or requested a maintainer to add) the necessary `Breaking*` or `New Feature` labels where relevant. - [x] I have done my best to ensure that my PR adheres to [the Fuel Labs Code Review Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md). - [x] I have requested a review from the relevant team or maintainers.
8 tasks
8 tasks
IGI-111
pushed a commit
that referenced
this issue
May 17, 2024
## Description This PR is part of #5727. Fix the issue of trying to solve expressions that were typed as `ErrorRecovery` inside `const_eval`. ## Checklist - [x] I have linked to any relevant issues. - [x] I have commented my code, particularly in hard-to-understand areas. - [x] I have updated the documentation where relevant (API docs, the reference, and the Sway book). - [ ] If my change requires substantial documentation changes, I have [requested support from the DevRel team](https://github.com/FuelLabs/devrel-requests/issues/new/choose) - [x] I have added tests that prove my fix is effective or that my feature works. - [x] I have added (or requested a maintainer to add) the necessary `Breaking*` or `New Feature` labels where relevant. - [x] I have done my best to ensure that my PR adheres to [the Fuel Labs Code Review Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md). - [x] I have requested a review from the relevant team or maintainers.
IGI-111
pushed a commit
that referenced
this issue
May 20, 2024
## Description This PR is part of #5727 and fixes a problem with AbiEncode/AbiDecode code generation. The problem lies in the fact that to encode/decode structs we need to "touch" all fields. Which makes all fields to pass DCA. Now, when a `StructFieldAccess` lives inside an autogenerated source, the "code flow graph" will be built as usual, but no edge will be created to the struct field, nor to the field owner struct declaration. ## Checklist - [x] I have linked to any relevant issues. - [x] I have commented my code, particularly in hard-to-understand areas. - [x] I have updated the documentation where relevant (API docs, the reference, and the Sway book). - [x] If my change requires substantial documentation changes, I have [requested support from the DevRel team](https://github.com/FuelLabs/devrel-requests/issues/new/choose) - [x] I have added tests that prove my fix is effective or that my feature works. - [x] I have added (or requested a maintainer to add) the necessary `Breaking*` or `New Feature` labels where relevant. - [x] I have done my best to ensure that my PR adheres to [the Fuel Labs Code Review Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md). - [x] I have requested a review from the relevant team or maintainers. --------- Co-authored-by: Joshua Batty <joshpbatty@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Now that #4769 has been implemented we should take the necessary steps to turn the experimental encoding into the default:
implicit_stdset tofalseand nocoreas dependency.MegaExample#5782Actions that needs to be undone after #5915 is merged:
Enable these tests again:
name-resolution-after-monomorphizationagain #5996)These seems to be a bug related with shadows and glob imports. These issues seem to point to similar issues: #5500, #5700
LDC is not working at the moment. As soon everything is fixed we need to enable and test this again.
Fix warnings of these tests
should_pass/dca/contract/superabi_contract_calls
What happens here is that when we implement a trait for Contract, we actually generate two functions: one prefixed __contract_entry that is called by the method selector; and another one normal, that can be called freely. So, if the trait method is never called manually, it is marked as dead.
should_pass/dca/unused_fields
auto-impl is making all fields being used. so no dead code warnings are being generated. We need to fix this.
fix dca for encoding v1 autogenerated code #6006
Nice to do for future versions:
1 - Contract method selector using match on string slices, ideally desugaring into radix matching;
2 - CEI analysis running normally on "__entry". We need CEI analysis to know about
Neveror be smart about control flow;3 - Improve the implementation to increase
Buffersize dynamically: Blocked by #59174 -
Bufferused by AbiEncode needs a push_bytes so we can be more efficient when encoding Bytes and others. This should decrease significantlygasusage in some cases.Encoding v1 for Configurables
1 - [ ] Avoid decoding configurable on each reference.
Small issue where
impl AbiEncode/Decodeis not in scope when calling contracts:#5936
From #5942
Numericintou64. This should be fixed.The text was updated successfully, but these errors were encountered: