Skip to content

Commit

Permalink
fix test
Browse files Browse the repository at this point in the history 
reset gas_price to 1_000 or diff between actual gas and encoded are too
high for small tx
  • Loading branch information
@pgherveou
pgherveou committed Jan 11, 2025
1 parent 50232db commit 0bb2c6f
Showing 1 changed file with 64 additions and 59 deletions.
123 changes: 64 additions & 59 deletions substrate/frame/revive/src/evm/runtime.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ type CallOf<T> = <T as frame_system::Config>::RuntimeCall;
/// - Not too high, ensuring the gas value is large enough (at least 7 digits) to encode the
/// ref_time, proof_size, and deposit into the less significant (6 lower) digits of the gas value.
/// - Not too low, enabling users to adjust the gas price to define a tip.
pub const GAS_PRICE: u32 = 100_000u32;
pub const GAS_PRICE: u32 = 1_000u32;

/// Convert a `Balance` into a gas value, using the fixed `GAS_PRICE`.
/// The gas is calculated as `balance / GAS_PRICE`, rounded up to the nearest integer.
Expand Down Expand Up @@ -324,6 +324,11 @@ pub trait EthExtra {
let GenericTransaction { nonce, chain_id, to, value, input, gas, gas_price, .. } =
GenericTransaction::from_signed(tx, None);

let Some(gas) = gas else {
log::debug!(target: LOG_TARGET, "No gas provided");
return Err(InvalidTransaction::Call);
};

if chain_id.unwrap_or_default() != <Self::Config as Config>::ChainId::get().into() {
log::debug!(target: LOG_TARGET, "Invalid chain_id {chain_id:?}");
return Err(InvalidTransaction::Call);
Expand All @@ -337,13 +342,11 @@ pub trait EthExtra {

let data = input.unwrap_or_default().0;

let (gas_limit, storage_deposit_limit) = <Self::Config as Config>::EthGasEncoder::decode(
gas.unwrap_or_default(),
)
.ok_or_else(|| {
log::debug!(target: LOG_TARGET, "Failed to decode gas: {gas:?}");
InvalidTransaction::Call
})?;
let (gas_limit, storage_deposit_limit) =
<Self::Config as Config>::EthGasEncoder::decode(gas).ok_or_else(|| {
log::debug!(target: LOG_TARGET, "Failed to decode gas: {gas:?}");
InvalidTransaction::Call
})?;

let call = if let Some(dest) = to {
crate::Call::call::<Self::Config> {
Expand Down Expand Up @@ -380,13 +383,13 @@ pub trait EthExtra {
// Fees calculated with the fixed `GAS_PRICE`
// When we dry-run the transaction, we set the gas to `Fee / GAS_PRICE`
let eth_fee_no_tip = U256::from(GAS_PRICE)
.saturating_mul(gas.unwrap_or_default())
.saturating_mul(gas)
.try_into()
.map_err(|_| InvalidTransaction::Call)?;

// Fees with the actual gas_price from the transaction.
let eth_fee: BalanceOf<Self::Config> = U256::from(gas_price.unwrap_or_default())
.saturating_mul(gas.unwrap_or_default())
.saturating_mul(gas)
.try_into()
.map_err(|_| InvalidTransaction::Call)?;

Expand All @@ -411,7 +414,8 @@ pub trait EthExtra {
}

if eth_fee_no_tip > actual_fee.saturating_mul(2u32.into()) {
log::debug!(target: LOG_TARGET, "actual fees: {actual_fee:?} too high, base eth fees: {eth_fee_no_tip:?}");
log::debug!(target: LOG_TARGET, "actual fees: {actual_fee:?} too high, base eth fees:
{eth_fee_no_tip:?}");
return Err(InvalidTransaction::Call.into())
}

Expand Down Expand Up @@ -473,8 +477,6 @@ mod test {
#[derive(Clone)]
struct UncheckedExtrinsicBuilder {
tx: GenericTransaction,
gas_limit: Weight,
storage_deposit_limit: BalanceOf<Test>,
before_validate: Option<std::sync::Arc<dyn Fn() + Send + Sync>>,
}

Expand All @@ -488,8 +490,6 @@ mod test {
gas_price: Some(U256::from(GAS_PRICE)),
..Default::default()
},
gas_limit: Weight::zero(),
storage_deposit_limit: 0,
before_validate: None,
}
}
Expand All @@ -506,11 +506,6 @@ mod test {
Ok(dry_run) => {
log::debug!(target: LOG_TARGET, "Estimated gas: {:?}", dry_run.eth_gas);
self.tx.gas = Some(dry_run.eth_gas);
let (gas_limit, deposit) =
<<Test as Config>::EthGasEncoder as GasEncoder<_>>::decode(dry_run.eth_gas)
.unwrap();
self.gas_limit = gas_limit;
self.storage_deposit_limit = deposit;
},
Err(err) => {
log::debug!(target: LOG_TARGET, "Failed to estimate gas: {:?}", err);
Expand All @@ -522,31 +517,35 @@ mod test {
fn call_with(dest: H160) -> Self {
let mut builder = Self::new();
builder.tx.to = Some(dest);
ExtBuilder::default().build().execute_with(|| builder.estimate_gas());
builder
}

/// Create a new builder with an instantiate call.
fn instantiate_with(code: Vec<u8>, data: Vec<u8>) -> Self {
let mut builder = Self::new();
builder.tx.input = Some(Bytes(code.into_iter().chain(data.into_iter()).collect()));
ExtBuilder::default().build().execute_with(|| builder.estimate_gas());
builder
}

/// Update the transaction with the given function.
fn update(mut self, f: impl FnOnce(&mut GenericTransaction) -> ()) -> Self {
f(&mut self.tx);
self
}
/// Set before_validate function.
fn before_validate(mut self, f: impl Fn() + Send + Sync + 'static) -> Self {
self.before_validate = Some(std::sync::Arc::new(f));
self
}

fn check(
self,
) -> Result<(RuntimeCall, SignedExtra, GenericTransaction), TransactionValidityError> {
self.mutate_estimate_and_check(Box::new(|_| ()))
}

/// Call `check` on the unchecked extrinsic, and `pre_dispatch` on the signed extension.
fn check(&self) -> Result<(RuntimeCall, SignedExtra), TransactionValidityError> {
fn mutate_estimate_and_check(
mut self,
f: Box<dyn FnOnce(&mut GenericTransaction) -> ()>,
) -> Result<(RuntimeCall, SignedExtra, GenericTransaction), TransactionValidityError> {
ExtBuilder::default().build().execute_with(|| self.estimate_gas());
f(&mut self.tx);
ExtBuilder::default().build().execute_with(|| {
let UncheckedExtrinsicBuilder { tx, before_validate, .. } = self.clone();

Expand All @@ -557,8 +556,9 @@ mod test {
100_000_000_000_000,
);

let payload =
account.sign_transaction(tx.try_into_unsigned().unwrap()).signed_payload();
let payload = account
.sign_transaction(tx.clone().try_into_unsigned().unwrap())
.signed_payload();
let call = RuntimeCall::Contracts(crate::Call::eth_transact { payload });

let encoded_len = call.encoded_size();
Expand All @@ -578,22 +578,26 @@ mod test {
0,
)?;

Ok((result.function, extra))
Ok((result.function, extra, tx))
})
}
}

#[test]
fn check_eth_transact_call_works() {
let builder = UncheckedExtrinsicBuilder::call_with(H160::from([1u8; 20]));
let (call, _, tx) = builder.check().unwrap();
let (gas_limit, storage_deposit_limit) =
<<Test as Config>::EthGasEncoder as GasEncoder<_>>::decode(tx.gas.unwrap()).unwrap();

assert_eq!(
builder.check().unwrap().0,
call,
crate::Call::call::<Test> {
dest: builder.tx.to.unwrap(),
value: builder.tx.value.unwrap_or_default().as_u64(),
gas_limit: builder.gas_limit,
storage_deposit_limit: builder.storage_deposit_limit,
data: builder.tx.input.unwrap_or_default().0
dest: tx.to.unwrap(),
value: tx.value.unwrap_or_default().as_u64(),
data: tx.input.unwrap_or_default().0,
gas_limit,
storage_deposit_limit
}
.into()
);
Expand All @@ -604,28 +608,30 @@ mod test {
let (code, _) = compile_module("dummy").unwrap();
let data = vec![];
let builder = UncheckedExtrinsicBuilder::instantiate_with(code.clone(), data.clone());
let (call, _, tx) = builder.check().unwrap();
let (gas_limit, storage_deposit_limit) =
<<Test as Config>::EthGasEncoder as GasEncoder<_>>::decode(tx.gas.unwrap()).unwrap();

assert_eq!(
builder.check().unwrap().0,
call,
crate::Call::instantiate_with_code::<Test> {
value: builder.tx.value.unwrap_or_default().as_u64(),
gas_limit: builder.gas_limit,
storage_deposit_limit: builder.storage_deposit_limit,
value: tx.value.unwrap_or_default().as_u64(),
code,
data,
salt: None
salt: None,
gas_limit,
storage_deposit_limit
}
.into()
);
}

#[test]
fn check_eth_transact_nonce_works() {
let builder = UncheckedExtrinsicBuilder::call_with(H160::from([1u8; 20]))
.update(|tx| tx.nonce = Some(1u32.into()));
let builder = UncheckedExtrinsicBuilder::call_with(H160::from([1u8; 20]));

assert_eq!(
builder.check(),
builder.mutate_estimate_and_check(Box::new(|tx| tx.nonce = Some(1u32.into()))),
Err(TransactionValidityError::Invalid(InvalidTransaction::Future))
);

Expand All @@ -642,11 +648,10 @@ mod test {

#[test]
fn check_eth_transact_chain_id_works() {
let builder = UncheckedExtrinsicBuilder::call_with(H160::from([1u8; 20]))
.update(|tx| tx.chain_id = Some(42.into()));
let builder = UncheckedExtrinsicBuilder::call_with(H160::from([1u8; 20]));

assert_eq!(
builder.check(),
builder.mutate_estimate_and_check(Box::new(|tx| tx.chain_id = Some(42.into()))),
Err(TransactionValidityError::Invalid(InvalidTransaction::Call))
);
}
Expand All @@ -659,7 +664,7 @@ mod test {

// Fail because the tx input fail to get the blob length
assert_eq!(
builder.clone().update(|tx| tx.input = Some(Bytes(vec![1, 2, 3]))).check(),
builder.mutate_estimate_and_check(Box::new(|tx| tx.input = Some(Bytes(vec![1, 2, 3])))),
Err(TransactionValidityError::Invalid(InvalidTransaction::Call))
);
}
Expand Down Expand Up @@ -691,27 +696,27 @@ mod test {
];

for (msg, update_tx, err) in scenarios {
let builder =
UncheckedExtrinsicBuilder::call_with(H160::from([1u8; 20])).update(update_tx);
let res = UncheckedExtrinsicBuilder::call_with(H160::from([1u8; 20]))
.mutate_estimate_and_check(update_tx);

assert_eq!(builder.check(), Err(TransactionValidityError::Invalid(err)), "{}", msg);
assert_eq!(res, Err(TransactionValidityError::Invalid(err)), "{}", msg);
}
}

#[test]
fn check_transaction_tip() {
let (code, _) = compile_module("dummy").unwrap();
let data = vec![];
let builder = UncheckedExtrinsicBuilder::instantiate_with(code.clone(), data.clone())
.update(|tx| {
tx.gas_price = Some(tx.gas_price.unwrap() * 103 / 100);
log::debug!(target: LOG_TARGET, "Gas price: {:?}", tx.gas_price);
});
let (_, extra, tx) =
UncheckedExtrinsicBuilder::instantiate_with(code.clone(), data.clone())
.mutate_estimate_and_check(Box::new(|tx| {
tx.gas_price = Some(tx.gas_price.unwrap() * 103 / 100);
log::debug!(target: LOG_TARGET, "Gas price: {:?}", tx.gas_price);
}))
.unwrap();

let tx = &builder.tx;
let expected_tip =
tx.gas_price.unwrap() * tx.gas.unwrap() - U256::from(GAS_PRICE) * tx.gas.unwrap();
let (_, extra) = builder.check().unwrap();
assert_eq!(U256::from(extra.1.tip()), expected_tip);
}
}

0 comments on commit 0bb2c6f

Please sign in to comment.