Skip to content

Commit

Permalink
Use rollback_to correctly.
Browse files Browse the repository at this point in the history
Removed `V2` gas costs version to follow the old way of compatibility.
Implemented `offset` logic in more easier way.
Fixed flakiness of the `test_poa_multiple_producers` test.
  • Loading branch information
xgreenx committed Dec 23, 2024
1 parent 2fc07bb commit 7fc47f1
Show file tree
Hide file tree
Showing 10 changed files with 62 additions and 182 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
- [2233](https://github.com/FuelLabs/fuel-core/pull/2233): Introduce a new column `modification_history_v2` for storing the modification history in the historical rocksDB. Keys in this column are stored in big endian order. Changed the behaviour of the historical rocksDB to write changes for new block heights to the new column, and to perform lookup of values from the `modification_history_v2` table first, and then from the `modification_history` table, performing a migration upon access if necessary.

#### Breaking
- [2438](https://github.com/FuelLabs/fuel-core/pull/2438): Updated `fuel-vm` to `0.59.1` release. Check [release notes](https://github.com/FuelLabs/fuel-vm/releases/tag/v0.59.0) for more details.
- [2389](https://github.com/FuelLabs/fuel-core/pull/2258): Updated the `messageProof` GraphQL schema to return a non-nullable `MessageProof`.
- [2154](https://github.com/FuelLabs/fuel-core/pull/2154): Transaction graphql endpoints use `TransactionType` instead of `fuel_tx::Transaction`.
- [2446](https://github.com/FuelLabs/fuel-core/pull/2446): Use graphiql instead of graphql-playground due to known vulnerability and stale development.
Expand Down
3 changes: 0 additions & 3 deletions benches/benches/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ where
let clock = quanta::Clock::new();

let original_db = vm.as_mut().database_mut().clone();
let original_memory = vm.memory().clone();
// During block production/validation for each state, which may affect the state of the database,
// we create a new storage transaction. The code here simulates the same behavior to have
// the same nesting level and the same performance.
Expand All @@ -53,7 +52,6 @@ where

let mut total = core::time::Duration::ZERO;
for _ in 0..iters {
vm.memory_mut().clone_from(&original_memory);
let start = black_box(clock.raw());
match instruction {
Instruction::CALL(call) => {
Expand All @@ -72,7 +70,6 @@ where
vm.as_mut().database_mut().reset_changes();
}
*vm.as_mut().database_mut() = original_db;
*vm.memory_mut() = original_memory;
total
})
});
Expand Down
11 changes: 3 additions & 8 deletions benches/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,6 @@ impl TryFrom<VmBench> for VmBenchPrepared {
let prepare_script = prepare_script
.into_iter()
.chain(iter::once(op::ret(RegId::ONE)))
.chain(iter::once(instruction))
.collect();

let mut tx = TransactionBuilder::script(prepare_script, data);
Expand Down Expand Up @@ -531,9 +530,7 @@ impl TryFrom<VmBench> for VmBenchPrepared {
}
}

let start_vm = vm.clone();
let original_db = vm.as_mut().database_mut().clone();
let original_memory = vm.memory().clone();
let vm_before_first_instruction = vm.clone();
let mut vm = vm.add_recording();
match instruction {
Instruction::CALL(call) => {
Expand All @@ -546,13 +543,11 @@ impl TryFrom<VmBench> for VmBenchPrepared {
}
let storage_diff = vm.storage_diff();
let mut vm = vm.remove_recording();
// TODO: Check if this is the intended use of rollback_to.
let mut diff = start_vm.rollback_to(&vm);
let mut diff = vm.rollback_to(&vm_before_first_instruction);
diff += storage_diff;
let diff: diff::Diff<diff::InitialVmState> = diff.into();
vm.reset_vm_state(&diff);
*vm.as_mut().database_mut() = original_db;
*vm.memory_mut() = original_memory;
assert_eq!(vm_before_first_instruction, vm);

Ok(Self {
vm,
Expand Down
1 change: 0 additions & 1 deletion crates/client/assets/schema.sdl
Original file line number Diff line number Diff line change
Expand Up @@ -477,7 +477,6 @@ type GasCosts {

enum GasCostsVersion {
V1
V2
}

type Genesis {
Expand Down
128 changes: 3 additions & 125 deletions crates/client/src/client/schema/chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,6 @@ pub struct GasCosts {
#[cynic(schema_path = "./assets/schema.sdl")]
pub enum GasCostsVersion {
V1,
V2,
#[cynic(fallback)]
Unknown,
}
Expand All @@ -353,7 +352,7 @@ impl TryFrom<GasCosts> for fuel_core_types::fuel_tx::GasCosts {
fn try_from(value: GasCosts) -> Result<Self, Self::Error> {
match value.version {
GasCostsVersion::V1 => Ok(fuel_core_types::fuel_tx::GasCosts::new(
fuel_core_types::fuel_tx::consensus_parameters::gas::GasCostsValuesV4 {
fuel_core_types::fuel_tx::consensus_parameters::gas::GasCostsValuesV5 {
add: value.add.into(),
addi: value.addi.into(),
and: value.and.into(),
Expand Down Expand Up @@ -439,6 +438,7 @@ impl TryFrom<GasCosts> for fuel_core_types::fuel_tx::GasCosts {
wqmm: value.wqmm.into(),
xor: value.xor.into(),
xori: value.xori.into(),
ecop: value.ecop.map(Into::into).unwrap_or(0),

aloc: value.aloc_dependent_cost.into(),
bsiz: value.bsiz.map(Into::into).unwrap_or(fuel_core_types::fuel_tx::consensus_parameters::DependentCost::free()),
Expand All @@ -464,136 +464,14 @@ impl TryFrom<GasCosts> for fuel_core_types::fuel_tx::GasCosts {
smo: value.smo.into(),
srwq: value.srwq.into(),
swwq: value.swwq.into(),
epar: value.epar.map(Into::into).unwrap_or(fuel_core_types::fuel_tx::consensus_parameters::DependentCost::free()),
contract_root: value.contract_root.into(),
state_root: value.state_root.into(),
vm_initialization: value.vm_initialization.into(),
new_storage_per_byte: value.new_storage_per_byte.into(),
}
.into(),
)),
GasCostsVersion::V2 => {
Ok(fuel_core_types::fuel_tx::GasCosts::new(
fuel_core_types::fuel_tx::consensus_parameters::gas::GasCostsValuesV5 {
add: value.add.into(),
addi: value.addi.into(),
and: value.and.into(),
andi: value.andi.into(),
bal: value.bal.into(),
bhei: value.bhei.into(),
bhsh: value.bhsh.into(),
burn: value.burn.into(),
cb: value.cb.into(),
cfsi: value.cfsi.into(),
div: value.div.into(),
divi: value.divi.into(),
eck1: value.eck1.into(),
ecr1: value.ecr1.into(),
eq: value.eq.into(),
exp: value.exp.into(),
expi: value.expi.into(),
flag: value.flag.into(),
gm: value.gm.into(),
gt: value.gt.into(),
gtf: value.gtf.into(),
ji: value.ji.into(),
jmp: value.jmp.into(),
jne: value.jne.into(),
jnei: value.jnei.into(),
jnzi: value.jnzi.into(),
jmpf: value.jmpf.into(),
jmpb: value.jmpb.into(),
jnzf: value.jnzf.into(),
jnzb: value.jnzb.into(),
jnef: value.jnef.into(),
jneb: value.jneb.into(),
lb: value.lb.into(),
log: value.log.into(),
lt: value.lt.into(),
lw: value.lw.into(),
mint: value.mint.into(),
mlog: value.mlog.into(),
mod_op: value.mod_op.into(),
modi: value.modi.into(),
move_op: value.move_op.into(),
movi: value.movi.into(),
mroo: value.mroo.into(),
mul: value.mul.into(),
muli: value.muli.into(),
mldv: value.mldv.into(),
noop: value.noop.into(),
not: value.not.into(),
or: value.or.into(),
ori: value.ori.into(),
poph: value.poph.into(),
popl: value.popl.into(),
pshh: value.pshh.into(),
pshl: value.pshl.into(),
ret: value.ret.into(),
rvrt: value.rvrt.into(),
sb: value.sb.into(),
sll: value.sll.into(),
slli: value.slli.into(),
srl: value.srl.into(),
srli: value.srli.into(),
srw: value.srw.into(),
sub: value.sub.into(),
subi: value.subi.into(),
sw: value.sw.into(),
sww: value.sww.into(),
time: value.time.into(),
tr: value.tr.into(),
tro: value.tro.into(),
wdcm: value.wdcm.into(),
wqcm: value.wqcm.into(),
wdop: value.wdop.into(),
wqop: value.wqop.into(),
wdml: value.wdml.into(),
wqml: value.wqml.into(),
wddv: value.wddv.into(),
wqdv: value.wqdv.into(),
wdmd: value.wdmd.into(),
wqmd: value.wqmd.into(),
wdam: value.wdam.into(),
wqam: value.wqam.into(),
wdmm: value.wdmm.into(),
wqmm: value.wqmm.into(),
xor: value.xor.into(),
xori: value.xori.into(),
ecop: value.ecop.map(Into::into).unwrap_or(0),

aloc: value.aloc_dependent_cost.into(),
bsiz: value.bsiz.map(Into::into).unwrap_or(fuel_core_types::fuel_tx::consensus_parameters::DependentCost::free()),
bldd: value.bldd.map(Into::into).unwrap_or(fuel_core_types::fuel_tx::consensus_parameters::DependentCost::free()),
cfe: value.cfe.into(),
cfei: value.cfei_dependent_cost.into(),
call: value.call.into(),
ccp: value.ccp.into(),
croo: value.croo.into(),
csiz: value.csiz.into(),
ed19: value.ed19_dependent_cost.into(),
k256: value.k256.into(),
ldc: value.ldc.into(),
logd: value.logd.into(),
mcl: value.mcl.into(),
mcli: value.mcli.into(),
mcp: value.mcp.into(),
mcpi: value.mcpi.into(),
meq: value.meq.into(),
retd: value.retd.into(),
s256: value.s256.into(),
scwq: value.scwq.into(),
smo: value.smo.into(),
srwq: value.srwq.into(),
swwq: value.swwq.into(),
epar: value.epar.map(Into::into).unwrap_or(fuel_core_types::fuel_tx::consensus_parameters::DependentCost::free()),
contract_root: value.contract_root.into(),
state_root: value.state_root.into(),
vm_initialization: value.vm_initialization.into(),
new_storage_per_byte: value.new_storage_per_byte.into(),
}
.into(),
))
}
_ => Err(ConversionError::UnknownVariant("GasCostsVersion")),
}
}
Expand Down
5 changes: 2 additions & 3 deletions crates/fuel-core/src/schema/chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ pub struct GasCosts(fuel_tx::GasCosts);
#[derive(Clone, Copy, Debug, Enum, Eq, PartialEq)]
pub enum GasCostsVersion {
V1,
V2,
}

#[derive(Clone, Copy, Debug, Enum, Eq, PartialEq)]
Expand Down Expand Up @@ -281,8 +280,8 @@ impl GasCosts {
GasCostsValues::V1(_)
| GasCostsValues::V2(_)
| GasCostsValues::V3(_)
| GasCostsValues::V4(_) => GasCostsVersion::V1,
GasCostsValues::V5(_) => GasCostsVersion::V2,
| GasCostsValues::V4(_)
| GasCostsValues::V5(_) => GasCostsVersion::V1,
}
}

Expand Down
24 changes: 15 additions & 9 deletions crates/fuel-core/src/state/rocks_db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -760,7 +760,7 @@ where
key: &[u8],
column: Self::Column,
offset: usize,
mut buf: &mut [u8],
buf: &mut [u8],
) -> StorageResult<Option<usize>> {
self.metrics.read_meter.inc();
let column_metrics = self.metrics.columns_read_statistic.get(&column.id());
Expand All @@ -771,15 +771,21 @@ where
.get_pinned_cf_opt(&self.cf(column), key, &self.read_options)
.map_err(|e| DatabaseError::Other(e.into()))?
.map(|value| {
if let Some(read) = value.len().checked_sub(offset) {
std::io::Write::write_all(&mut buf, value[offset..].as_ref())
.map_err(|e| DatabaseError::Other(anyhow::anyhow!(e)))?;
StorageResult::Ok(read)
} else {
Err(StorageError::Other(anyhow::anyhow!(
"Offset is out of bounds"
)))
let bytes_len = value.len();
let start = offset;
let end = offset.saturating_add(buf.len());

if end > bytes_len {
return Err(StorageError::Other(anyhow::anyhow!(
"Offset `{offset}` is out of bounds `{bytes_len}` \
for key `{:?}` and column `{column:?}`",
key
)));
}

let starting_from_offset = &value[start..end];
buf[..].copy_from_slice(starting_from_offset);
Ok(buf.len())
})
.transpose()?;

Expand Down
16 changes: 10 additions & 6 deletions crates/services/txpool_v2/src/tests/mocks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,13 +134,17 @@ impl StorageRead<BlobData> for MockDb {

bytes
.map(|bytes| {
if let Some(len) = bytes.0.len().checked_sub(offset) {
// SAFETY: offset is guaranteed to be less or equal to value.len() by the check above
buf.copy_from_slice(&bytes.0[offset..]);
Ok(len)
} else {
Err(())
let bytes_len = bytes.as_ref().len();
let start = offset;
let end = offset.saturating_add(buf.len());

if end > bytes_len {
return Err(());
}

let starting_from_offset = &bytes.as_ref()[start..end];
buf[..].copy_from_slice(starting_from_offset);
Ok(buf.len())
})
.transpose()
}
Expand Down
32 changes: 15 additions & 17 deletions crates/storage/src/kv_store.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
//! The module provides plain abstract definition of the key-value store.
use crate::{
Error as StorageError,
Result as StorageResult,
};
use crate::Result as StorageResult;

#[cfg(feature = "alloc")]
use alloc::{
Expand Down Expand Up @@ -72,20 +69,21 @@ pub trait KeyValueInspect {
) -> StorageResult<Option<usize>> {
self.get(key, column)?
.map(|value| {
if let Some(read) = value.len().checked_sub(offset) {
if read != buf.len() {
return Err(StorageError::Other(anyhow::anyhow!(
"Buffer size is not equal to the value size"
)));
};
// SAFETY: offset is guaranteed to be less or equal to value.len() by the if let check above
buf.copy_from_slice(value[offset..].as_ref());
Ok(buf.len())
} else {
Err(StorageError::Other(anyhow::anyhow!(
"Offset is out of bounds"
)))
let bytes_len = value.as_ref().len();
let start = offset;
let end = offset.saturating_add(buf.len());

if end > bytes_len {
return Err(anyhow::anyhow!(
"Offset `{offset}` is out of bounds `{bytes_len}` for key `{:?}`",
key
)
.into());
}

let starting_from_offset = &value.as_ref()[start..end];
buf[..].copy_from_slice(starting_from_offset);
Ok(buf.len())
})
.transpose()
}
Expand Down
Loading

0 comments on commit 7fc47f1

Please sign in to comment.