Skip to content

Commit

Permalink
Merge branch 'roman-cketh-units' into 'master'
Browse files Browse the repository at this point in the history
chore(cketh): use proper units for gas and fees

This change introduces proper type-level units for gas and fee values. 

See merge request dfinity-lab/public/ic!15233
  • Loading branch information
roman-kashitsyn committed Oct 5, 2023
2 parents 9d9ed88 + 30b59b0 commit f784eb2
Show file tree
Hide file tree
Showing 12 changed files with 155 additions and 134 deletions.
29 changes: 21 additions & 8 deletions rs/ethereum/cketh/minter/src/checked_amount.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,26 @@ use std::ops::Rem;
/// ```
/// use ic_cketh_minter::checked_amount::CheckedAmountOf;
///
/// enum MetricApple{}
/// enum MetricApple {}
/// type Apples = CheckedAmountOf<MetricApple>;
///
/// enum MetricOrange {}
/// type Oranges = CheckedAmountOf<MetricOrange>;
///
/// enum OrangesPerAppleUnit {}
/// type OrangesPerApple = CheckedAmountOf<OrangesPerAppleUnit>;
///
/// let three_apples = Apples::from(3_u8);
///
/// //Checked addition
/// // Checked addition
/// assert_eq!(three_apples.checked_add(Apples::TWO), Some(Apples::from(5_u8)));
/// assert_eq!(Apples::MAX.checked_add(Apples::ONE), None);
///
/// //Checked subtraction
/// // Checked subtraction
/// assert_eq!(three_apples.checked_sub(Apples::TWO), Some(Apples::ONE));
/// assert_eq!(Apples::TWO.checked_sub(three_apples), None);
///
/// //Checked multiplication by scalar
/// // Checked multiplication by scalar
/// assert_eq!(three_apples.checked_mul(2_u8), Some(Apples::from(6_u8)));
/// assert_eq!(Apples::MAX.checked_mul(2_u8), None);
///
Expand Down Expand Up @@ -58,6 +65,10 @@ impl<Unit> CheckedAmountOf<Unit> {
Self(value, PhantomData)
}

pub const fn into_inner(self) -> ethnum::u256 {
self.0
}

#[inline]
pub const fn from_words(hi: u128, lo: u128) -> Self {
Self::from_inner(ethnum::u256::from_words(hi, lo))
Expand Down Expand Up @@ -91,10 +102,12 @@ impl<Unit> CheckedAmountOf<Unit> {
self.0.checked_sub(other.0).map(Self::from_inner)
}

pub fn checked_mul<T: Into<ethnum::u256>>(self, other: T) -> Option<Self> {
self.0
.checked_mul(other.into())
.map(|res| Self(res, PhantomData))
pub fn change_units<NewUnits>(self) -> CheckedAmountOf<NewUnits> {
CheckedAmountOf::<NewUnits>::from_inner(self.0)
}

pub fn checked_mul<T: Into<ethnum::u256>>(self, factor: T) -> Option<Self> {
self.0.checked_mul(factor.into()).map(Self::from_inner)
}

pub fn checked_div_ceil<T: Into<ethnum::u256>>(self, rhs: T) -> Option<Self> {
Expand Down
3 changes: 1 addition & 2 deletions rs/ethereum/cketh/minter/src/endpoints.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use crate::eth_rpc::into_nat;
use crate::transactions::EthWithdrawalRequest;
use crate::tx::{SignedEip1559TransactionRequest, TransactionPrice};
use candid::{CandidType, Deserialize, Nat};
Expand All @@ -18,7 +17,7 @@ pub struct Eip1559TransactionPrice {
impl From<TransactionPrice> for Eip1559TransactionPrice {
fn from(value: TransactionPrice) -> Self {
Self {
gas_limit: into_nat(value.gas_limit),
gas_limit: value.gas_limit.into(),
max_fee_per_gas: value.max_fee_per_gas.into(),
max_priority_fee_per_gas: value.max_priority_fee_per_gas.into(),
max_transaction_fee: value.max_transaction_fee().into(),
Expand Down
6 changes: 3 additions & 3 deletions rs/ethereum/cketh/minter/src/eth_rpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use crate::endpoints::CandidBlockTag;
use crate::eth_rpc_client::responses::TransactionReceipt;
use crate::eth_rpc_error::{sanitize_send_raw_transaction_result, Parser};
use crate::logs::{DEBUG, TRACE_HTTP};
use crate::numeric::{BlockNumber, LogIndex, TransactionCount, Wei};
use crate::numeric::{BlockNumber, LogIndex, TransactionCount, Wei, WeiPerGas};
use crate::state::{mutate_state, State};
use candid::{candid_method, CandidType, Principal};
use ethnum;
Expand Down Expand Up @@ -354,9 +354,9 @@ pub struct FeeHistory {
/// This includes the next block after the newest of the returned range,
/// because this value can be derived from the newest block.
/// Zeroes are returned for pre-EIP-1559 blocks.
pub base_fee_per_gas: Vec<Wei>,
pub base_fee_per_gas: Vec<WeiPerGas>,
/// A two-dimensional array of effective priority fees per gas at the requested block percentiles.
pub reward: Vec<Vec<Wei>>,
pub reward: Vec<Vec<WeiPerGas>>,
}

impl HttpResponsePayload for FeeHistory {
Expand Down
10 changes: 5 additions & 5 deletions rs/ethereum/cketh/minter/src/eth_rpc_client/responses.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::eth_rpc::{Hash, HttpResponsePayload, Quantity, ResponseTransform};
use crate::numeric::{BlockNumber, Wei};
use crate::eth_rpc::{Hash, HttpResponsePayload, ResponseTransform};
use crate::numeric::{BlockNumber, GasAmount, WeiPerGas};
use minicbor::{Decode, Encode};
use serde::{Deserialize, Serialize};
use std::fmt::{Display, Formatter};
Expand All @@ -17,11 +17,11 @@ pub struct TransactionReceipt {

/// The total base charge plus tip paid for each unit of gas
#[n(2)]
pub effective_gas_price: Wei,
pub effective_gas_price: WeiPerGas,

/// The amount of gas used by this specific transaction alone
#[cbor(n(3), with = "crate::cbor::u256")]
pub gas_used: Quantity,
#[n(3)]
pub gas_used: GasAmount,

/// Status of the transaction.
#[n(4)]
Expand Down
8 changes: 4 additions & 4 deletions rs/ethereum/cketh/minter/src/eth_rpc_client/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -227,9 +227,9 @@ mod multi_call_results {
}

mod eth_get_transaction_receipt {
use crate::eth_rpc::{Hash, Quantity};
use crate::eth_rpc::Hash;
use crate::eth_rpc_client::responses::{TransactionReceipt, TransactionStatus};
use crate::numeric::{BlockNumber, Wei};
use crate::numeric::{BlockNumber, GasAmount, WeiPerGas};
use assert_matches::assert_matches;
use proptest::proptest;
use std::str::FromStr;
Expand Down Expand Up @@ -263,8 +263,8 @@ mod eth_get_transaction_receipt {
)
.unwrap(),
block_number: BlockNumber::new(0x4132ec),
effective_gas_price: Wei::new(0xfefbee3e),
gas_used: Quantity::new(0x5208),
effective_gas_price: WeiPerGas::new(0xfefbee3e),
gas_used: GasAmount::new(0x5208),
status: TransactionStatus::Success,
transaction_hash: Hash::from_str(
"0x0e59bd032b9b22aca5e2784e4cf114783512db00988c716cf17a1cc755a0a93d"
Expand Down
14 changes: 14 additions & 0 deletions rs/ethereum/cketh/minter/src/numeric/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ use phantom_newtype::Id;
pub enum WeiTag {}
pub type Wei = CheckedAmountOf<WeiTag>;

pub enum WeiPerGasUnit {}
pub type WeiPerGas = CheckedAmountOf<WeiPerGasUnit>;

pub fn wei_from_milli_ether(value: u128) -> Wei {
const MILLI_ETHER: u64 = 1_000_000_000_000_000_000;
Wei::new(value)
Expand All @@ -34,10 +37,21 @@ pub type TransactionCount = CheckedAmountOf<TransactionCountTag>;
pub enum BlockNumberTag {}
pub type BlockNumber = CheckedAmountOf<BlockNumberTag>;

pub enum GasUnit {}
/// The number of gas units attached to a transaction for execution.
pub type GasAmount = CheckedAmountOf<GasUnit>;

pub enum EthLogIndexTag {}
pub type LogIndex = CheckedAmountOf<EthLogIndexTag>;
pub enum BurnIndexTag {}
pub type LedgerBurnIndex = Id<BurnIndexTag, u64>;

pub enum MintIndexTag {}
pub type LedgerMintIndex = Id<MintIndexTag, u64>;

impl CheckedAmountOf<WeiPerGasUnit> {
pub fn transaction_cost(self, gas: GasAmount) -> Option<Wei> {
self.checked_mul(gas.into_inner())
.map(|value| value.change_units())
}
}
2 changes: 1 addition & 1 deletion rs/ethereum/cketh/minter/src/state/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,7 @@ prop_compose! {
nonce in arb_checked_amount_of(),
max_priority_fee_per_gas in arb_checked_amount_of(),
max_fee_per_gas in arb_checked_amount_of(),
gas_limit in arb_u256(),
gas_limit in arb_checked_amount_of(),
destination in arb_address(),
amount in arb_checked_amount_of(),
data in pvec(any::<u8>(), 0..20),
Expand Down
53 changes: 26 additions & 27 deletions rs/ethereum/cketh/minter/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -233,8 +233,7 @@ fn address_display() {

mod rlp_encoding {
use crate::address::Address;
use crate::eth_rpc::Quantity;
use crate::numeric::{TransactionNonce, Wei};
use crate::numeric::{GasAmount, TransactionNonce, Wei, WeiPerGas};
use crate::tx::{
AccessList, Eip1559Signature, Eip1559TransactionRequest, SignedEip1559TransactionRequest,
};
Expand Down Expand Up @@ -342,9 +341,9 @@ mod rlp_encoding {
let transaction = Eip1559TransactionRequest {
chain_id: SEPOLIA_TEST_CHAIN_ID,
nonce: TransactionNonce::from(6_u8),
max_priority_fee_per_gas: Wei::new(0x59682f00),
max_fee_per_gas: Wei::new(0x598653cd),
gas_limit: Quantity::new(56_511),
max_priority_fee_per_gas: WeiPerGas::new(0x59682f00),
max_fee_per_gas: WeiPerGas::new(0x598653cd),
gas_limit: GasAmount::new(56_511),
destination: Address::from_str("0xb44B5e756A894775FC32EDdf3314Bb1B1944dC34").unwrap(),
amount: Wei::new(1_000_000_000_000_000),
data: hex::decode(
Expand Down Expand Up @@ -723,7 +722,7 @@ mod eth_get_block_by_number {

mod eth_fee_history {
use crate::eth_rpc::{BlockSpec, BlockTag, FeeHistory, FeeHistoryParams, Quantity};
use crate::numeric::{BlockNumber, Wei};
use crate::numeric::{BlockNumber, WeiPerGas};

#[test]
fn should_serialize_fee_history_params_as_tuple() {
Expand Down Expand Up @@ -791,38 +790,38 @@ mod eth_fee_history {
FeeHistory {
oldest_block: BlockNumber::new(0x10f73fc),
base_fee_per_gas: vec![
Wei::new(0x729d3f3b3),
Wei::new(0x766e503ea),
Wei::new(0x75b51b620),
Wei::new(0x74094f2b4),
Wei::new(0x716724f03),
Wei::new(0x73b467f76)
WeiPerGas::new(0x729d3f3b3),
WeiPerGas::new(0x766e503ea),
WeiPerGas::new(0x75b51b620),
WeiPerGas::new(0x74094f2b4),
WeiPerGas::new(0x716724f03),
WeiPerGas::new(0x73b467f76)
],
reward: vec![
vec![
Wei::new(0x5f5e100),
Wei::new(0x5f5e100),
Wei::new(0x68e7780)
WeiPerGas::new(0x5f5e100),
WeiPerGas::new(0x5f5e100),
WeiPerGas::new(0x68e7780)
],
vec![
Wei::new(0x55d4a80),
Wei::new(0x5f5e100),
Wei::new(0x5f5e100)
WeiPerGas::new(0x55d4a80),
WeiPerGas::new(0x5f5e100),
WeiPerGas::new(0x5f5e100)
],
vec![
Wei::new(0x5f5e100),
Wei::new(0x5f5e100),
Wei::new(0x5f5e100)
WeiPerGas::new(0x5f5e100),
WeiPerGas::new(0x5f5e100),
WeiPerGas::new(0x5f5e100)
],
vec![
Wei::new(0x5f5e100),
Wei::new(0x5f5e100),
Wei::new(0x5f5e100)
WeiPerGas::new(0x5f5e100),
WeiPerGas::new(0x5f5e100),
WeiPerGas::new(0x5f5e100)
],
vec![
Wei::new(0x5f5e100),
Wei::new(0x5f5e100),
Wei::new(0x180789e0)
WeiPerGas::new(0x5f5e100),
WeiPerGas::new(0x5f5e100),
WeiPerGas::new(0x180789e0)
]
],
}
Expand Down
7 changes: 3 additions & 4 deletions rs/ethereum/cketh/minter/src/transactions/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -227,8 +227,7 @@ impl EthTransactions {
// If transaction count at block height H is c > 0, then transactions with nonces
// 0, 1, ..., c - 1 were mined. If transaction count is 0, then no transactions were mined.
// The nonce of the first pending transaction is then exactly c.
let first_pending_tx_nonce =
TransactionNonce::from_be_bytes(latest_transaction_count.to_be_bytes());
let first_pending_tx_nonce: TransactionNonce = latest_transaction_count.change_units();
let mut transactions_to_resubmit = Vec::new();
for (nonce, burn_index, signed_tx) in self
.sent_tx
Expand Down Expand Up @@ -328,8 +327,8 @@ impl EthTransactions {
&self,
finalized_transaction_count: &TransactionCount,
) -> BTreeMap<Hash, LedgerBurnIndex> {
let first_non_finalized_tx_nonce =
TransactionNonce::from_be_bytes(finalized_transaction_count.to_be_bytes());
let first_non_finalized_tx_nonce: TransactionNonce =
finalized_transaction_count.change_units();
let mut transactions = BTreeMap::new();
for (_nonce, index, sent_txs) in self
.sent_tx
Expand Down
Loading

0 comments on commit f784eb2

Please sign in to comment.