Skip to content

Commit

Permalink
fix size of configurable buffer for enums (#6366)
Browse files Browse the repository at this point in the history
## Description

This PR fixes an issue where the encoded bytes for enums were not big
enough for the biggest variant.

## Checklist

- [ ] I have linked to any relevant issues.
- [x] 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)
- [x] 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.
- [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: Sophie Dankel <47993817+sdankel@users.noreply.github.com>
  • Loading branch information
xunilrj and sdankel authored Aug 7, 2024
1 parent 93656fe commit d5aa2d6
Show file tree
Hide file tree
Showing 6 changed files with 91 additions and 29 deletions.
14 changes: 13 additions & 1 deletion sway-core/src/ir_generation/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -322,11 +322,23 @@ pub(crate) fn compile_configurables(
let opt_metadata = md_mgr.span_to_md(context, &decl.span);

if context.experimental.new_encoding {
let encoded_bytes = match constant.value {
let mut encoded_bytes = match constant.value {
ConstantValue::RawUntypedSlice(bytes) => bytes,
_ => unreachable!(),
};

let config_type_info = engines.te().get(decl.type_ascription.type_id);
let buffer_size = match config_type_info.abi_encode_size_hint(engines) {
crate::AbiEncodeSizeHint::Exact(len) => len,
crate::AbiEncodeSizeHint::Range(_, len) => len,
_ => unreachable!("unexpected type accepted as configurable"),
};

if buffer_size > encoded_bytes.len() {
encoded_bytes.extend([0].repeat(buffer_size - encoded_bytes.len()));
}
assert!(encoded_bytes.len() == buffer_size);

let decode_fn = engines.de().get(decl.decode_fn.as_ref().unwrap().id());
let decode_fn = cache.ty_function_decl_to_unique_function(
engines,
Expand Down
43 changes: 40 additions & 3 deletions test/src/e2e_vm_tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use fuel_vm::fuel_tx;
use fuel_vm::fuel_types::canonical::Input;
use fuel_vm::prelude::*;
use regex::Regex;
use std::collections::HashSet;
use std::collections::{BTreeMap, HashSet};
use std::io::stdout;
use std::io::Write;
use std::str::FromStr;
Expand Down Expand Up @@ -336,6 +336,41 @@ impl TestContext {
run_and_capture_output(|| harness::compile_to_bytes(&name, &run_config)).await;
*output = out;

if let Ok(result) = result.as_ref() {
let packages = match result {
forc_pkg::Built::Package(p) => [p.clone()].to_vec(),
forc_pkg::Built::Workspace(p) => p.clone(),
};

for p in packages {
let bytecode_len = p.bytecode.bytes.len();

let configurables = match &p.program_abi {
sway_core::asm_generation::ProgramABI::Fuel(abi) => {
abi.configurables.as_ref().cloned().unwrap_or_default()
}
sway_core::asm_generation::ProgramABI::Evm(_)
| sway_core::asm_generation::ProgramABI::MidenVM(_) => vec![],
}
.into_iter()
.map(|x| (x.offset, x.name))
.collect::<BTreeMap<u64, String>>();

let mut items = configurables.iter().peekable();
while let Some(current) = items.next() {
let next_offset = match items.peek() {
Some(next) => *next.0,
None => bytecode_len as u64,
};
let size = next_offset - current.0;
output.push_str(&format!(
"Configurable Encoded Bytes Buffer Size: {} {}\n",
current.1, size
));
}
}
}

check_file_checker(checker, &name, output)?;

let compiled = result?;
Expand Down Expand Up @@ -913,7 +948,7 @@ fn parse_test_toml(path: &Path, run_config: &RunConfig) -> Result<TestDescriptio
}

// if new encoding is on, allow a "category_new_encoding"
// for tests that should have difference categories
// for tests that should have different categories
let category = if run_config.experimental.new_encoding {
toml_content
.get("category_new_encoding")
Expand Down Expand Up @@ -956,7 +991,9 @@ fn parse_test_toml(path: &Path, run_config: &RunConfig) -> Result<TestDescriptio
}

// To allow different configs, we must ignore file check if it is not fail
if category != TestCategory::FailsToCompile {
if toml_content.get("category_new_encoding").is_some()
&& category != TestCategory::FailsToCompile
{
file_check = FileCheck("".into());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
"typeArguments": null
},
"name": "BOOL",
"offset": 6968
"offset": 7184
},
{
"configurableType": {
Expand All @@ -16,7 +16,7 @@
"typeArguments": null
},
"name": "U8",
"offset": 7112
"offset": 7376
},
{
"configurableType": {
Expand All @@ -25,7 +25,7 @@
"typeArguments": null
},
"name": "ANOTHER_U8",
"offset": 6896
"offset": 7112
},
{
"configurableType": {
Expand All @@ -34,7 +34,7 @@
"typeArguments": null
},
"name": "U16",
"offset": 7056
"offset": 7320
},
{
"configurableType": {
Expand All @@ -43,7 +43,7 @@
"typeArguments": null
},
"name": "U32",
"offset": 7096
"offset": 7360
},
{
"configurableType": {
Expand All @@ -52,7 +52,7 @@
"typeArguments": null
},
"name": "U64",
"offset": 7104
"offset": 7368
},
{
"configurableType": {
Expand All @@ -61,7 +61,7 @@
"typeArguments": null
},
"name": "U256",
"offset": 7064
"offset": 7328
},
{
"configurableType": {
Expand All @@ -70,7 +70,7 @@
"typeArguments": null
},
"name": "B256",
"offset": 6936
"offset": 7152
},
{
"configurableType": {
Expand All @@ -79,7 +79,7 @@
"typeArguments": []
},
"name": "CONFIGURABLE_STRUCT",
"offset": 7008
"offset": 7272
},
{
"configurableType": {
Expand All @@ -88,7 +88,7 @@
"typeArguments": []
},
"name": "CONFIGURABLE_ENUM_A",
"offset": 6976
"offset": 7192
},
{
"configurableType": {
Expand All @@ -97,7 +97,7 @@
"typeArguments": []
},
"name": "CONFIGURABLE_ENUM_B",
"offset": 6992
"offset": 7232
},
{
"configurableType": {
Expand All @@ -106,7 +106,7 @@
"typeArguments": null
},
"name": "ARRAY_BOOL",
"offset": 6904
"offset": 7120
},
{
"configurableType": {
Expand All @@ -115,7 +115,7 @@
"typeArguments": null
},
"name": "ARRAY_U64",
"offset": 6912
"offset": 7128
},
{
"configurableType": {
Expand All @@ -124,7 +124,7 @@
"typeArguments": null
},
"name": "TUPLE_BOOL_U64",
"offset": 7040
"offset": 7304
},
{
"configurableType": {
Expand All @@ -133,7 +133,7 @@
"typeArguments": null
},
"name": "STR_4",
"offset": 7032
"offset": 7296
},
{
"configurableType": {
Expand All @@ -142,7 +142,7 @@
"typeArguments": null
},
"name": "NOT_USED",
"offset": 7024
"offset": 7288
}
],
"encoding": "1",
Expand Down Expand Up @@ -231,6 +231,11 @@
"name": "B",
"type": 12,
"typeArguments": null
},
{
"name": "C",
"type": 4,
"typeArguments": null
}
],
"type": "enum ConfigurableEnum",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ struct ConfigurableStruct {
enum ConfigurableEnum {
A: bool,
B: u64,
C: b256
}

impl core::ops::Eq for ConfigurableEnum {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,19 @@ expected_result_new_encoding = { action = "return_data", value = "" }
validate_abi = true
expected_warnings = 1
unsupported_profiles = ["debug"]

# check: $()ANOTHER_U8 8
# check: $()ARRAY_BOOL 8
# check: $()ARRAY_U64 24
# check: $()B256 32
# check: $()BOOL 8
# check: $()CONFIGURABLE_ENUM_A 40
# check: $()CONFIGURABLE_ENUM_B 40
# check: $()CONFIGURABLE_STRUCT 16
# check: $()NOT_USED 8
# check: $()STR_4 8
# check: $()TUPLE_BOOL_U64 16
# check: $()U16 8
# check: $()U256 32
# check: $()U32 8
# check: $()U64 8
Original file line number Diff line number Diff line change
@@ -1,12 +1,3 @@
category = "compile"

# check: $()#[namespace(my_namespace)]
# nextln: $()Attribute namespace is deprecated.
# nextln: $()You can use namespaces inside storage:
# nextln: $()storage {
# nextln: $()my_storage_namespace {
# nextln: $()field: u64 = 1,
# nextln: $()}
# nextln: $()}

expected_warnings = 2

0 comments on commit d5aa2d6

Please sign in to comment.