Skip to content

Commit

Permalink
Fix last iteration in sequential opcode (#2479)
Browse files Browse the repository at this point in the history
Fixes #2022

Previously the read and write of the last key of the contract state was
creating error because of multiple bad checks of limits. Also the key
was increase in all case at the end of the loop causing the `increase()`
to fail on overflow even if there is values to be inserted left.

## Checklist
- [x] Breaking changes are clearly marked as such in the PR description
and changelog
- [x] New behavior is reflected in tests
- [x] [The specification](https://github.com/FuelLabs/fuel-specs/)
matches the implemented behavior (link update PR if changes are needed)

### Before requesting review
- [x] I have reviewed the code myself
- [x] I have created follow-up issues caused by this PR and linked them
here

---------

Co-authored-by: Green Baneling <[email protected]>
  • Loading branch information
AurelienFT and xgreenx authored Dec 13, 2024
1 parent 2ea9cd4 commit 825e18d
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 9 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
- [2369](https://github.com/FuelLabs/fuel-core/pull/2369): The `transaction_insertion_time_in_thread_pool_milliseconds` metric is properly collected.
- [2413](https://github.com/FuelLabs/fuel-core/issues/2413): block production immediately errors if unable to lock the mutex.
- [2389](https://github.com/FuelLabs/fuel-core/pull/2389): Fix construction of reverse iterator in RocksDB.
- [2479](https://github.com/FuelLabs/fuel-core/pull/2479): Fix an error on the last iteration of the read and write sequential opcodes on contract storage.
- [2478](https://github.com/FuelLabs/fuel-core/pull/2478): Fix proof created by `message_receipts_proof` function by ignoring the receipts from failed transactions to match `message_outbox_root`.
- [2485](https://github.com/FuelLabs/fuel-core/pull/2485): Hardcode the timestamp of the genesis block and version of `tai64` to avoid breaking changes for us.

Expand Down
22 changes: 13 additions & 9 deletions crates/storage/src/vm_storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -344,11 +344,13 @@ where
let mut state_key = Bytes32::zeroed();

let mut results = Vec::new();
for _ in 0..range {
for i in 0..range {
if i != 0 {
key.increase()?;
}
key.to_big_endian(state_key.as_mut());
let multikey = ContractsStateKey::new(contract_id, &state_key);
results.push(self.database.storage::<ContractsState>().get(&multikey)?);
key.increase()?;
}
Ok(results)
}
Expand All @@ -367,12 +369,15 @@ where

// verify key is in range
current_key
.checked_add(U256::from(values.len()))
.checked_add(U256::from(values.len().saturating_sub(1)))
.ok_or_else(|| anyhow!("range op exceeded available keyspace"))?;

let mut key_bytes = Bytes32::zeroed();
let mut found_unset = 0u32;
for value in values {
for (idx, value) in values.iter().enumerate() {
if idx != 0 {
current_key.increase()?;
}
current_key.to_big_endian(key_bytes.as_mut());

let option = self
Expand All @@ -385,8 +390,6 @@ where
.checked_add(1)
.expect("We've checked it above via `values.len()`");
}

current_key.increase()?;
}

Ok(found_unset as usize)
Expand All @@ -403,7 +406,10 @@ where
let mut current_key = U256::from_big_endian(start_key.as_ref());

let mut key_bytes = Bytes32::zeroed();
for _ in 0..range {
for i in 0..range {
if i != 0 {
current_key.increase()?;
}
current_key.to_big_endian(key_bytes.as_mut());

let option = self
Expand All @@ -412,8 +418,6 @@ where
.take(&(contract_id, &key_bytes).into())?;

found_unset |= option.is_none();

current_key.increase()?;
}

if found_unset {
Expand Down
15 changes: 15 additions & 0 deletions tests/tests/vm_storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,11 @@ mod tests {
=> Err(())
; "read fails on partially initialized range if keyspace exceeded"
)]
#[test_case(
&[(*u256_to_bytes32(U256::MAX), vec![0; 32])], *u256_to_bytes32(U256::MAX), 1
=> Ok(vec![Some(vec![0; 32])])
; "read success when reading last key"
)]
fn read_sequential_range(
prefilled_slots: &[([u8; 32], Vec<u8>)],
start_key: [u8; 32],
Expand Down Expand Up @@ -161,6 +166,11 @@ mod tests {
=> Err(())
; "insert fails if start_key + range > u256::MAX"
)]
#[test_case(
&[(*u256_to_bytes32(U256::MAX), vec![0; 32])], *u256_to_bytes32(U256::MAX), &[vec![1; 32]]
=> Ok(true)
; "try to modify only the last value of storage"
)]
fn insert_range(
prefilled_slots: &[([u8; 32], Vec<u8>)],
start_key: [u8; 32],
Expand Down Expand Up @@ -255,6 +265,11 @@ mod tests {
=> (vec![], false)
; "remove multiple slots over partially uninitialized middle range"
)]
#[test_case(
&[(*u256_to_bytes32(U256::MAX), vec![0; 32])], *u256_to_bytes32(U256::MAX), 1
=> (vec![], true)
; "try to modify only the last value of storage"
)]
fn remove_range(
prefilled_slots: &[([u8; 32], Vec<u8>)],
start_key: [u8; 32],
Expand Down

0 comments on commit 825e18d

Please sign in to comment.