-
Notifications
You must be signed in to change notification settings - Fork 5.4k
Add experimental ABI alias metadata support. #7261
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
base: master
Are you sure you want to change the base?
Conversation
1a6a2fb to
5989ba2
Compare
CodSpeed Performance ReportMerging #7261 will not alter performanceComparing Summary
|
381c53f to
9b7242e
Compare
9b7242e to
f0d3478
Compare
f0d3478 to
66db5dc
Compare
66db5dc to
e565e43
Compare
e565e43 to
75d1794
Compare
75d1794 to
b7251ef
Compare
b7251ef to
909004a
Compare
909004a to
3be6772
Compare
3be6772 to
4880b44
Compare
4880b44 to
136282a
Compare
ab8183d to
4e1dbf5
Compare
4e1dbf5 to
c4b5c5b
Compare
c4b5c5b to
13f96b6
Compare
4e237c6 to
23270b5
Compare
4478960 to
b056aef
Compare
b056aef to
fdcd1a6
Compare
fdcd1a6 to
1f39f64
Compare
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 PR is being reviewed by Cursor Bugbot
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
Bug: Array Type ID Inconsistency Causes ABI Errors
Array element type IDs use raw initial_type_id.index() instead of the metadata ID from the generated declaration. This is inconsistent with the refactoring applied to enum variants, struct fields, and tuple elements, and can produce incorrect type IDs in the ABI JSON when type caching is enabled, potentially causing type ID mismatches or references to non-existent metadata types.
sway-core/src/abi_generation/fuel_abi.rs#L1268-L1283
sway/sway-core/src/abi_generation/fuel_abi.rs
Lines 1268 to 1283 in 1f39f64
| // `program_abi::TypeApplication` for the array element type | |
| Some(vec![program_abi::TypeApplication { | |
| name: "__array_element".to_string(), | |
| type_id: program_abi::TypeId::Metadata(MetadataTypeId( | |
| elem_ty.initial_type_id.index(), | |
| )), | |
| error_message: None, | |
| type_arguments: elem_ty.initial_type_id.get_abi_type_arguments( | |
| handler, | |
| ctx, | |
| engines, | |
| metadata_types, | |
| concrete_types, | |
| elem_ty.type_id, | |
| metadata_types_to_add, | |
| )?, |
Bug: Resolve Slice Type ID Metadata Inconsistency
Slice element type IDs use raw initial_type_id.index() instead of the metadata ID from the generated declaration. This is inconsistent with the refactoring applied to enum variants, struct fields, and tuple elements, and can produce incorrect type IDs in the ABI JSON when type caching is enabled, potentially causing type ID mismatches or references to non-existent metadata types.
sway-core/src/abi_generation/fuel_abi.rs#L1304-L1319
sway/sway-core/src/abi_generation/fuel_abi.rs
Lines 1304 to 1319 in 1f39f64
| // `program_abi::TypeApplication` for the array element type | |
| Some(vec![program_abi::TypeApplication { | |
| name: "__slice_element".to_string(), | |
| type_id: program_abi::TypeId::Metadata(MetadataTypeId( | |
| elem_ty.initial_type_id.index(), | |
| )), | |
| error_message: None, | |
| type_arguments: elem_ty.initial_type_id.get_abi_type_arguments( | |
| handler, | |
| ctx, | |
| engines, | |
| metadata_types, | |
| concrete_types, | |
| elem_ty.type_id, | |
| metadata_types_to_add, | |
| )?, |
| @@ -2723,7 +2724,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" | |||
| checksum = "8d162beedaa69905488a8da94f5ac3edb4dd4788b732fadb7bd120b2625c1976" | |||
| dependencies = [ | |||
| "data-encoding", | |||
| "syn 2.0.106", | |||
| "syn 1.0.109", | |||
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.
Bug: Dependency Version Mismatch Threatens Stability.
The data-encoding-macro-internal dependency downgrades from syn 2.0.106 to syn 1.0.109, which is a major version regression. This is inconsistent with the rest of the codebase where syn is being upgraded to 2.0.109, and could cause compilation failures or API incompatibilities since syn 1.x and 2.x have breaking changes between them.
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 was done by the resolve step of cargo update itself so not sure if it should be touched.
|
The issues being flagged by the review bot are from pre-existing code that I've not touched. |
Description
Bump the toolchain to
fuel-abi-types 0.16and point everyfuels-*dependency at the matching branch so codegen and tests consume the alias-aware ABI schema (Cargo.toml:82,Cargo.toml:264).Restructure the Fuel ABI emitter so both metadata and concrete type declarations share the same builders and run through
AbiContextcaches, preventing duplicate structs/enums in the serialized ABI (sway-core/src/abi_generation/fuel_abi.rs:42).Plumb the experimental
abi_type_aliasestoggle fromforc-pkginto ABI string generation and metadata collection so alias names survive resolution without being classified as generics (forc-pkg/src/pkg.rs:1811,sway-features/src/lib.rs:173,sway-core/src/type_system/id.rs:266,sway-core/src/types/collect_types_metadata.rs:128).Teach
TypeInfo::Aliasto either inline or preserve the alias name depending on the feature, and add validation so ABI names remain unique/valid before being emitted (sway-core/src/abi_generation/abi_str.rs:11,sway-core/src/abi_generation/fuel_abi.rs:167).Add an
abi_with_alias_experimentalcontract plus oracle JSONs for both encoding modes to lock in the expected output while the feature flag is on (test/src/e2e_vm_tests/test_programs/should_pass/test_contracts/abi_with_alias_experimental/Forc.toml:1), along with snapshot tweaks where alias strings changed (test/src/e2e_vm_tests/test_programs/should_fail/attributes_invalid_args/stdout.snap:1).Commits
fuel-abi-types.generate_type_metadata_declarationingenerate_concrete_type_declaration.Fixes #6564.
There is a matching PR for Rust SDK support at: FuelLabs/fuels-rs#1677
Checklist
Breaking*orNew Featurelabels where relevant.Note
Introduce an experimental abi_type_aliases feature and refactor ABI emission to preserve type aliases with caching/dedup, updating deps and tests accordingly.
experimental.abi_type_aliasesis enabled; emitaliasOflinks and keep alias names in type strings.abi_type_aliasesthrough forc and metadata collection (logs/panic), enabling alias-aware JSON generation.abi_type_aliasesexperimental feature (incl.#[cfg]support) and document it in the book.fuel-abi-types 0.16; patchfuels-*crates to matching branch.Written by Cursor Bugbot for commit 1f39f64. This will update automatically on new commits. Configure here.