Skip to content

Commit

Permalink
Fixed contracts tables to touch only own SMT nodes (#1536)
Browse files Browse the repository at this point in the history 
It was possible to remove the SMT nodes in the foreign contract because
it was not prefixed with the `ContractId`. The caller could use the same
storage key/asset ID with exactly the same values to delete the SMT
nodes in another contract.

Fixes #1535

---------

Co-authored-by: Hannes Karppila <hannes.karppila@gmail.com>
Co-authored-by: Brandon Vrooman <brandon.vrooman@fuel.sh>
  • Loading branch information
@xgreenx @Dentosal
3 people authored Dec 11, 2023
1 parent 9c526c4 commit 673c91d
Show file tree
Hide file tree
Showing 3 changed files with 100 additions and 27 deletions.
7 changes: 6 additions & 1 deletion CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,12 @@ Description of the upcoming release here.
### Changed

- [#1517](https://github.com/FuelLabs/fuel-core/pull/1517): Changed default gossip heartbeat interval to 500ms.
- [#1520](https://github.com/FuelLabs/fuel-core/pull/1520): Extract `executor` into `fuel-core-executor` crate.
- [#1520](https://github.com/FuelLabs/fuel-core/pull/1520): Extract `executor` into `fuel-core-executor` crate.

### Fixed

#### Breaking
- [#1536](https://github.com/FuelLabs/fuel-core/pull/1536): The change fixes the contracts tables to not touch SMT nodes of foreign contracts. Before, it was possible to invalidate the SMT from another contract. It is a breaking change and requires re-calculating the whole state from the beginning with new SMT roots.


## [Version 0.21.0]
Expand Down
60 changes: 47 additions & 13 deletions crates/fuel-core/src/database/balances.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,9 @@ use fuel_core_types::{
},
};
use itertools::Itertools;
use std::{
borrow::{
BorrowMut,
Cow,
},
ops::Deref,
use std::borrow::{
BorrowMut,
Cow,
};

impl StorageInspect<ContractsAssets> for Database {
Expand Down Expand Up @@ -85,10 +82,9 @@ impl StorageMutate<ContractsAssets> for Database {
MerkleTree::load(storage, &root)
.map_err(|err| StorageError::Other(anyhow::anyhow!("{err:?}")))?;

let asset_id = *key.asset_id().deref();
// Update the contact's key-value dataset. The key is the asset id and the
// value the Word
tree.update(MerkleTreeKey::new(asset_id), value.to_be_bytes().as_slice())
tree.update(MerkleTreeKey::new(key), value.to_be_bytes().as_slice())
.map_err(|err| StorageError::Other(anyhow::anyhow!("{err:?}")))?;

// Generate new metadata for the updated tree
Expand Down Expand Up @@ -121,10 +117,9 @@ impl StorageMutate<ContractsAssets> for Database {
MerkleTree::load(storage, &root)
.map_err(|err| StorageError::Other(anyhow::anyhow!("{err:?}")))?;

let asset_id = *key.asset_id().deref();
// Update the contract's key-value dataset. The key is the asset id and
// the value is the Word
tree.delete(MerkleTreeKey::new(asset_id))
tree.delete(MerkleTreeKey::new(key))
.map_err(|err| StorageError::Other(anyhow::anyhow!("{err:?}")))?;

let root = tree.root();
Expand Down Expand Up @@ -190,9 +185,12 @@ impl Database {
// Merkle data:
// - Asset key should be converted into `MerkleTreeKey` by `new` function that hashes them.
// - The balance value are original.
let balances = balances
.into_iter()
.map(|(asset, value)| (MerkleTreeKey::new(asset), value.to_be_bytes()));
let balances = balances.into_iter().map(|(asset, value)| {
(
MerkleTreeKey::new(ContractsAssetKey::new(contract_id, &asset)),
value.to_be_bytes(),
)
});
let (root, nodes) = in_memory::MerkleTree::nodes_from_set(balances);
self.batch_insert(ContractsAssetsMerkleData::column(), nodes.into_iter())?;
let metadata = SparseMerkleMetadata { root };
Expand Down Expand Up @@ -438,6 +436,42 @@ mod tests {
assert_eq!(root_0, root_2);
}

#[test]
fn updating_foreign_contract_does_not_affect_the_given_contract_insertion() {
let given_contract_id = ContractId::from([1u8; 32]);
let foreign_contract_id = ContractId::from([2u8; 32]);
let database = &mut Database::default();

let asset_id = AssetId::new([0u8; 32]);
let balance: Word = 100;

// Given
let given_contract_key = (&given_contract_id, &asset_id).into();
let foreign_contract_key = (&foreign_contract_id, &asset_id).into();
database
.storage::<ContractsAssets>()
.insert(&given_contract_key, &balance)
.unwrap();

// When
database
.storage::<ContractsAssets>()
.insert(&foreign_contract_key, &balance)
.unwrap();
database
.storage::<ContractsAssets>()
.remove(&foreign_contract_key)
.unwrap();

// Then
let result = database
.storage::<ContractsAssets>()
.insert(&given_contract_key, &balance)
.unwrap();

assert!(result.is_some());
}

#[test]
fn init_contract_balances_works() {
use rand::{
Expand Down
60 changes: 47 additions & 13 deletions crates/fuel-core/src/database/state.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,9 @@ use fuel_core_types::{
},
};
use itertools::Itertools;
use std::{
borrow::{
BorrowMut,
Cow,
},
ops::Deref,
use std::borrow::{
BorrowMut,
Cow,
};

impl StorageInspect<ContractsState> for Database {
Expand Down Expand Up @@ -84,10 +81,9 @@ impl StorageMutate<ContractsState> for Database {
MerkleTree::load(storage, &root)
.map_err(|err| StorageError::Other(anyhow::anyhow!("{err:?}")))?;

let state_key = *key.state_key().deref();
// Update the contract's key-value dataset. The key is the state key and
// the value is the 32 bytes
tree.update(MerkleTreeKey::new(state_key), value.as_slice())
tree.update(MerkleTreeKey::new(key), value.as_slice())
.map_err(|err| StorageError::Other(anyhow::anyhow!("{err:?}")))?;

// Generate new metadata for the updated tree
Expand Down Expand Up @@ -120,10 +116,9 @@ impl StorageMutate<ContractsState> for Database {
MerkleTree::load(storage, &root)
.map_err(|err| StorageError::Other(anyhow::anyhow!("{err:?}")))?;

let state_key = *key.state_key().deref();
// Update the contract's key-value dataset. The key is the state key and
// the value is the 32 bytes
tree.delete(MerkleTreeKey::new(state_key))
tree.delete(MerkleTreeKey::new(key))
.map_err(|err| StorageError::Other(anyhow::anyhow!("{err:?}")))?;

let root = tree.root();
Expand Down Expand Up @@ -190,9 +185,12 @@ impl Database {
// Merkle data:
// - State key should be converted into `MerkleTreeKey` by `new` function that hashes them.
// - The state value are original.
let slots = slots
.into_iter()
.map(|(key, value)| (MerkleTreeKey::new(key), value));
let slots = slots.into_iter().map(|(key, value)| {
(
MerkleTreeKey::new(ContractsStateKey::new(contract_id, &key)),
value,
)
});
let (root, nodes) = in_memory::MerkleTree::nodes_from_set(slots);
self.batch_insert(ContractsStateMerkleData::column(), nodes.into_iter())?;
let metadata = SparseMerkleMetadata { root };
Expand Down Expand Up @@ -433,6 +431,42 @@ mod tests {
assert_eq!(root_0, root_2);
}

#[test]
fn updating_foreign_contract_does_not_affect_the_given_contract_insertion() {
let given_contract_id = ContractId::from([1u8; 32]);
let foreign_contract_id = ContractId::from([2u8; 32]);
let database = &mut Database::default();

let state_key = Bytes32::new([1u8; 32]);
let state_value = Bytes32::from([0xff; 32]);

// Given
let given_contract_key = (&given_contract_id, &state_key).into();
let foreign_contract_key = (&foreign_contract_id, &state_key).into();
database
.storage::<ContractsState>()
.insert(&given_contract_key, &state_value)
.unwrap();

// When
database
.storage::<ContractsState>()
.insert(&foreign_contract_key, &state_value)
.unwrap();
database
.storage::<ContractsState>()
.remove(&foreign_contract_key)
.unwrap();

// Then
let result = database
.storage::<ContractsState>()
.insert(&given_contract_key, &state_value)
.unwrap();

assert!(result.is_some());
}

#[test]
fn init_contract_state_works() {
use rand::{
Expand Down

0 comments on commit 673c91d

Please sign in to comment.