Skip to content

Commit

Permalink
fix(da_compression): invalid decompression of utxo id and CoinConfig …
Browse files Browse the repository at this point in the history
…fix (#2593)

## Linked Issues/PRs
<!-- List of related issues/PRs -->
some investigation related to why transaction ids were mismatching
before compression and after decompression

## Description
<!-- List of detailed changes -->
- Fixed CoinConfig generate function to create appropriate tx_id and
output_index
- Fixed utxo id lookup
- da compression integ test asserts tx ids are the same

AI generated:

### Code Organization:

*
[`crates/chain-config/src/config/coin.rs`](diffhunk://#diff-942b8fa635eb049417e89ef88024e08422c209c2952e0f5612e45531ee03a8b1R124-R171):
Moved `CoinConfigGenerator` to a new `coin_config_helpers` module and
updated its implementation to use a `CoinCount` type.
* Updated references to `CoinConfigGenerator` across multiple files to
use the new `coin_config_helpers` module.
[[1]](diffhunk://#diff-942b8fa635eb049417e89ef88024e08422c209c2952e0f5612e45531ee03a8b1L170-R182)
[[2]](diffhunk://#diff-942b8fa635eb049417e89ef88024e08422c209c2952e0f5612e45531ee03a8b1L183-R195)
[[3]](diffhunk://#diff-beed0a09969286877b6b2569d2e3775a9526d5328bfebe9ef04da44e16dc8492L49-R49)
[[4]](diffhunk://#diff-aceecd6cf4d806ddbb7fe3edc5d757cc292c1c8b81f9df2601cdc2103cca16f2R5-L6)
[[5]](diffhunk://#diff-e83ddeaff43cbea45b7b2a6c5b688054cf32fbcd7c5010fc5059d0812b1f413eR3-L4)
[[6]](diffhunk://#diff-12758c6f607876b04a2fd230ac5b6c16c275c65e82390f867404ddb62345119aR27-L28)
[[7]](diffhunk://#diff-12758c6f607876b04a2fd230ac5b6c16c275c65e82390f867404ddb62345119aL527-R527)

### Decompression Logic:

*
[`crates/compression/src/decompress.rs`](diffhunk://#diff-d0b271dacdbc77c5808aab48fee0fa85f338d593e1adc25e8bd0205c370dd824L71-R93):
Modified the decompression logic to handle mint transactions correctly
and ensure the TxPointer is set appropriately.
[[1]](diffhunk://#diff-d0b271dacdbc77c5808aab48fee0fa85f338d593e1adc25e8bd0205c370dd824L71-R93)
[[2]](diffhunk://#diff-d0b271dacdbc77c5808aab48fee0fa85f338d593e1adc25e8bd0205c370dd824R231-R232)

### Testing and Configuration:

*
[`tests/tests/da_compression.rs`](diffhunk://#diff-b7fc0c13495dc1a6096e1822748a1d512e84510d50d2ac1d56b1f9a839646342R66-R92):
Enhanced the test for fetching DA compressed blocks from GraphQL by
adding additional checks and ensuring the transactions' IDs match.
[[1]](diffhunk://#diff-b7fc0c13495dc1a6096e1822748a1d512e84510d50d2ac1d56b1f9a839646342R66-R92)
[[2]](diffhunk://#diff-b7fc0c13495dc1a6096e1822748a1d512e84510d50d2ac1d56b1f9a839646342R129-R157)

### Miscellaneous:

* Various files: Updated imports and removed unnecessary `where` clauses
to clean up the codebase.
[[1]](diffhunk://#diff-942b8fa635eb049417e89ef88024e08422c209c2952e0f5612e45531ee03a8b1L23)
[[2]](diffhunk://#diff-d0b271dacdbc77c5808aab48fee0fa85f338d593e1adc25e8bd0205c370dd824R21)
[[3]](diffhunk://#diff-d0b271dacdbc77c5808aab48fee0fa85f338d593e1adc25e8bd0205c370dd824R39)
[[4]](diffhunk://#diff-715d143d62233118cd663c107e340e6d32aeb6446c02c167893656316ce13cbeL276-R276)
[[5]](diffhunk://#diff-715d143d62233118cd663c107e340e6d32aeb6446c02c167893656316ce13cbeL304)
[[6]](diffhunk://#diff-715d143d62233118cd663c107e340e6d32aeb6446c02c167893656316ce13cbeL317-R321)


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

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

### After merging, notify other teams

[Add or remove entries as needed]

- [ ] [Rust SDK](https://github.com/FuelLabs/fuels-rs/)
- [ ] [Sway compiler](https://github.com/FuelLabs/sway/)
- [ ] [Platform
documentation](https://github.com/FuelLabs/devrel-requests/issues/new?assignees=&labels=new+request&projects=&template=NEW-REQUEST.yml&title=%5BRequest%5D%3A+)
(for out-of-organization contributors, the person merging the PR will do
this)
- [ ] Someone else?
  • Loading branch information
rymnc authored Jan 22, 2025
1 parent 9e3dd3f commit 4207d94
Show file tree
Hide file tree
Showing 10 changed files with 129 additions and 66 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
### Fixed
- [2609](https://github.com/FuelLabs/fuel-core/pull/2609): Check response before trying to deserialize, return error instead
- [2599](https://github.com/FuelLabs/fuel-core/pull/2599): Use the proper `url` apis to construct full url path in `BlockCommitterHttpApi` client
- [2593](https://github.com/FuelLabs/fuel-core/pull/2593): Fixed utxo id decompression

## [Version 0.41.0]

Expand Down
96 changes: 58 additions & 38 deletions crates/chain-config/src/config/coin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ use fuel_core_types::{
BlockHeight,
Bytes32,
},
fuel_vm::SecretKey,
};
use serde::{
Deserialize,
Expand Down Expand Up @@ -75,41 +74,6 @@ impl From<CoinConfig> for TableEntry<Coins> {
}
}

/// Generates a new coin config with a unique utxo id for testing
#[derive(Default, Debug)]
pub struct CoinConfigGenerator {
count: usize,
}

impl CoinConfigGenerator {
pub fn new() -> Self {
Self { count: 0 }
}

pub fn generate(&mut self) -> CoinConfig {
let mut bytes = [0u8; 32];
bytes[..std::mem::size_of::<usize>()].copy_from_slice(&self.count.to_be_bytes());

let config = CoinConfig {
tx_id: Bytes32::from(bytes),
..Default::default()
};
self.count = self.count.checked_add(1).expect("Max coin count reached");

config
}

pub fn generate_with(&mut self, secret: SecretKey, amount: u64) -> CoinConfig {
let owner = Address::from(*secret.public_key().hash());

CoinConfig {
amount,
owner,
..self.generate()
}
}
}

impl CoinConfig {
pub fn utxo_id(&self) -> UtxoId {
UtxoId::new(self.tx_id, self.output_index)
Expand Down Expand Up @@ -157,6 +121,62 @@ impl GenesisCommitment for Coin {
}
}

#[cfg(feature = "test-helpers")]
pub mod coin_config_helpers {
use crate::CoinConfig;
use fuel_core_types::{
fuel_types::{
Address,
Bytes32,
},
fuel_vm::SecretKey,
};

type CoinCount = u16;

/// Generates a new coin config with a unique utxo id for testing
#[derive(Default, Debug)]
pub struct CoinConfigGenerator {
count: CoinCount,
}

pub fn tx_id(count: CoinCount) -> Bytes32 {
let mut bytes = [0u8; 32];
bytes[..size_of::<CoinCount>()].copy_from_slice(&count.to_be_bytes());
bytes.into()
}

impl CoinConfigGenerator {
pub fn new() -> Self {
Self { count: 0 }
}

pub fn generate(&mut self) -> CoinConfig {
let tx_id = tx_id(self.count);

let config = CoinConfig {
tx_id,
output_index: self.count,
..Default::default()
};

self.count = self.count.checked_add(1).expect("Max coin count reached");

config
}

pub fn generate_with(&mut self, secret: SecretKey, amount: u64) -> CoinConfig {
let owner = Address::from(*secret.public_key().hash());

CoinConfig {
amount,
owner,
..self.generate()
}
}
}
}

#[cfg(test)]
mod tests {
use super::*;
Expand All @@ -167,7 +187,7 @@ mod tests {

#[test]
fn test_generate_unique_utxo_id() {
let mut generator = CoinConfigGenerator::new();
let mut generator = coin_config_helpers::CoinConfigGenerator::new();
let config1 = generator.generate();
let config2 = generator.generate();

Expand All @@ -180,7 +200,7 @@ mod tests {
let secret = SecretKey::random(&mut rng);
let amount = 1000;

let mut generator = CoinConfigGenerator::new();
let mut generator = coin_config_helpers::CoinConfigGenerator::new();
let config = generator.generate_with(secret, amount);

assert_eq!(config.owner, Address::from(*secret.public_key().hash()));
Expand Down
2 changes: 1 addition & 1 deletion crates/chain-config/src/config/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ use serde::{
use crate::SnapshotMetadata;

#[cfg(feature = "test-helpers")]
use crate::CoinConfigGenerator;
use crate::coin_config_helpers::CoinConfigGenerator;
#[cfg(feature = "test-helpers")]
use bech32::{
ToBase32,
Expand Down
23 changes: 21 additions & 2 deletions crates/compression/src/decompress.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use fuel_core_types::{
RegistryKey,
},
fuel_tx::{
field::TxPointer,
input::{
coin::{
Coin,
Expand All @@ -35,6 +36,7 @@ use fuel_core_types::{
Mint,
ScriptCode,
Transaction,
TxPointer as FuelTxPointer,
UtxoId,
},
fuel_types::{
Expand Down Expand Up @@ -68,12 +70,29 @@ where
db,
};

let transactions = <Vec<Transaction> as DecompressibleBy<_>>::decompress_with(
let mut transactions = <Vec<Transaction> as DecompressibleBy<_>>::decompress_with(
block.transactions(),
&ctx,
)
.await?;

let transaction_count = transactions.len();

// patch mint transaction
let mint_tx = transactions
.last_mut()
.ok_or_else(|| anyhow::anyhow!("No transactions"))?;
if let Transaction::Mint(mint) = mint_tx {
let tx_pointer = mint.tx_pointer_mut();
*tx_pointer = FuelTxPointer::new(
block.consensus_header().height,
#[allow(clippy::arithmetic_side_effects)]
u16::try_from(transaction_count - 1)?,
);
} else {
anyhow::bail!("Last transaction is not a mint");
}

Ok(PartialFuelBlock {
header: block.partial_block_header(),
transactions,
Expand Down Expand Up @@ -211,7 +230,7 @@ where
ctx: &DecompressCtx<D>,
) -> anyhow::Result<Self> {
Ok(Transaction::mint(
Default::default(), // TODO: what should this we do with this?
Default::default(), // TODO: what should we do with this?
c.input_contract.decompress(ctx).await?,
c.output_contract.decompress(ctx).await?,
c.mint_amount.decompress(ctx).await?,
Expand Down
21 changes: 8 additions & 13 deletions crates/fuel-core/src/graphql_api/da_compression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -273,10 +273,7 @@ impl_temporal_registry!(ContractId);
impl_temporal_registry!(ScriptCode);
impl_temporal_registry!(PredicateCode);

impl<'a, Tx> UtxoIdToPointer for CompressDbTx<'a, Tx>
where
Tx: OffChainDatabaseTransaction,
{
impl<'a, Tx> UtxoIdToPointer for CompressDbTx<'a, Tx> {
fn lookup(
&self,
utxo_id: fuel_core_types::fuel_tx::UtxoId,
Expand All @@ -301,7 +298,6 @@ where

impl<'a, Tx, Onchain> HistoryLookup for DecompressDbTx<'a, Tx, Onchain>
where
Tx: OffChainDatabaseTransaction,
Onchain: StorageInspect<Coins, Error = fuel_core_storage::Error>
+ StorageInspect<Messages, Error = fuel_core_storage::Error>
+ StorageInspect<FuelBlocks, Error = fuel_core_storage::Error>,
Expand All @@ -310,17 +306,16 @@ where
&self,
c: fuel_core_types::fuel_tx::CompressedUtxoId,
) -> anyhow::Result<fuel_core_types::fuel_tx::UtxoId> {
#[cfg(feature = "test-helpers")]
if c.tx_pointer.block_height() == 0u32.into() {
// This is a genesis coin, which is handled differently.
// See CoinConfigGenerator::generate which generates the genesis coins.
let mut bytes = [0u8; 32];
let tx_index = c.tx_pointer.tx_index();
bytes[..std::mem::size_of_val(&tx_index)]
.copy_from_slice(&tx_index.to_be_bytes());
return Ok(fuel_core_types::fuel_tx::UtxoId::new(
fuel_core_types::fuel_tx::TxId::from(bytes),
0,
));
let tx_id =
fuel_core_chain_config::coin_config_helpers::tx_id(c.output_index);

let utxo_id = fuel_core_types::fuel_tx::UtxoId::new(tx_id, c.output_index);

return Ok(utxo_id);
}

let block_info = self
Expand Down
4 changes: 2 additions & 2 deletions crates/fuel-core/src/p2p_test_helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
use crate::{
chain_config::{
coin_config_helpers::CoinConfigGenerator,
CoinConfig,
CoinConfigGenerator,
},
combined_database::CombinedDatabase,
database::{
Expand Down Expand Up @@ -246,7 +246,7 @@ pub async fn make_nodes(
let initial_coin = CoinConfig {
// set idx to prevent overlapping utxo_ids when
// merging with existing coins from config
output_index: 2,
output_index: 10,
..coin_generator.generate_with(secret, 10000)
};
let tx = TransactionBuilder::script(
Expand Down
4 changes: 2 additions & 2 deletions tests/tests/balances.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use fuel_core::{
chain_config::{
coin_config_helpers::CoinConfigGenerator,
CoinConfig,
CoinConfigGenerator,
MessageConfig,
StateConfig,
},
Expand Down Expand Up @@ -334,9 +334,9 @@ async fn first_5_balances() {
mod pagination {
use fuel_core::{
chain_config::{
coin_config_helpers::CoinConfigGenerator,
ChainConfig,
CoinConfig,
CoinConfigGenerator,
MessageConfig,
StateConfig,
},
Expand Down
2 changes: 1 addition & 1 deletion tests/tests/coin.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use fuel_core::{
chain_config::{
coin_config_helpers::CoinConfigGenerator,
CoinConfig,
CoinConfigGenerator,
StateConfig,
},
database::Database,
Expand Down
4 changes: 2 additions & 2 deletions tests/tests/coins.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ use rand::{
mod coin {
use super::*;
use fuel_core::chain_config::{
coin_config_helpers::CoinConfigGenerator,
ChainConfig,
CoinConfigGenerator,
};
use fuel_core_client::client::types::CoinType;
use fuel_core_types::fuel_crypto::SecretKey;
Expand Down Expand Up @@ -524,7 +524,7 @@ mod message_coin {

// It is combination of coins and deposit coins test cases.
mod all_coins {
use fuel_core::chain_config::CoinConfigGenerator;
use fuel_core::chain_config::coin_config_helpers::CoinConfigGenerator;
use fuel_core_client::client::types::CoinType;
use fuel_core_types::blockchain::primitives::DaBlockHeight;

Expand Down
Loading

0 comments on commit 4207d94

Please sign in to comment.