Skip to content

Commit

Permalink
fix(rpc v6 and v7): conversion from core.ResourceL1DataGas unsupported (
Browse files Browse the repository at this point in the history 
#2575)

* fix(rpcv7): conversion from core.ResourceL1DataGas unsupported

* fix(rpcv7): Add test

* fix(rpcv6): conversion from core.ResourceL1DataGas unsupported

* fix: make format
  • Loading branch information
@hudem1
hudem1 authored Feb 27, 2025
1 parent 94071e4 commit e354149
Show file tree
Hide file tree
Showing 5 changed files with 124 additions and 48 deletions.
5 changes: 5 additions & 0 deletions rpc/v6/transaction.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,11 @@ func adaptBroadcastedTransaction(broadcastedTxn *BroadcastedTransaction,
func adaptResourceBounds(rb map[core.Resource]core.ResourceBounds) map[Resource]ResourceBounds {
rpcResourceBounds := make(map[Resource]ResourceBounds)
for resource, bounds := range rb {
// ResourceL1DataGas is not supported in v6
if resource == core.ResourceL1DataGas {
continue
}

rpcResourceBounds[Resource(resource)] = ResourceBounds{
MaxAmount: new(felt.Felt).SetUint64(bounds.MaxAmount),
MaxPricePerUnit: bounds.MaxPricePerUnit,
Expand Down
34 changes: 34 additions & 0 deletions rpc/v6/transaction_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -790,6 +790,40 @@ func TestAddTransactionUnmarshal(t *testing.T) {
}
}

func TestAdaptTransaction(t *testing.T) {
t.Run("core.Resource `ResourceL1DataGas` should be ignored when converted to v6.Resource", func(t *testing.T) {
coreTx := core.InvokeTransaction{
Version: new(core.TransactionVersion).SetUint64(3),
ResourceBounds: map[core.Resource]core.ResourceBounds{
core.ResourceL1Gas: {MaxAmount: 1, MaxPricePerUnit: new(felt.Felt).SetUint64(2)},
core.ResourceL2Gas: {MaxAmount: 3, MaxPricePerUnit: new(felt.Felt).SetUint64(4)},
core.ResourceL1DataGas: {MaxAmount: 5, MaxPricePerUnit: new(felt.Felt).SetUint64(6)},
},
}

tx := rpc.AdaptTransaction(&coreTx)

expectedTx := &rpc.Transaction{
Type: rpc.TxnInvoke,
Version: new(felt.Felt).SetUint64(3),
ResourceBounds: &map[rpc.Resource]rpc.ResourceBounds{
rpc.ResourceL1Gas: {MaxAmount: new(felt.Felt).SetUint64(1), MaxPricePerUnit: new(felt.Felt).SetUint64(2)},
rpc.ResourceL2Gas: {MaxAmount: new(felt.Felt).SetUint64(3), MaxPricePerUnit: new(felt.Felt).SetUint64(4)},
},
Tip: new(felt.Felt).SetUint64(0),
// Those 4 fields are pointers to slice (the SliceHeader is allocated, it just refers to a nil array)
Signature: new([]*felt.Felt),
CallData: new([]*felt.Felt),
PaymasterData: new([]*felt.Felt),
AccountDeploymentData: new([]*felt.Felt),
NonceDAMode: utils.Ptr(rpc.DAModeL1),
FeeDAMode: utils.Ptr(rpc.DAModeL1),
}

require.Equal(t, expectedTx, tx)
})
}

func TestAddTransaction(t *testing.T) {
n := utils.Ptr(utils.Integration)
gw := adaptfeeder.New(feeder.NewTestClient(t, n))
Expand Down
9 changes: 7 additions & 2 deletions rpc/v7/transaction.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,11 @@ func adaptBroadcastedTransaction(broadcastedTxn *BroadcastedTransaction,
func adaptResourceBounds(rb map[core.Resource]core.ResourceBounds) map[Resource]ResourceBounds {
rpcResourceBounds := make(map[Resource]ResourceBounds)
for resource, bounds := range rb {
// ResourceL1DataGas is not supported in v7
if resource == core.ResourceL1DataGas {
continue
}

rpcResourceBounds[Resource(resource)] = ResourceBounds{
MaxAmount: new(felt.Felt).SetUint64(bounds.MaxAmount),
MaxPricePerUnit: bounds.MaxPricePerUnit,
Expand Down Expand Up @@ -726,7 +731,7 @@ func AdaptTransaction(t core.Transaction) *Transaction {
case *core.DeclareTransaction:
txn = adaptDeclareTransaction(v)
case *core.DeployAccountTransaction:
txn = adaptDeployAccountTrandaction(v)
txn = adaptDeployAccountTransaction(v)
case *core.L1HandlerTransaction:
nonce := v.Nonce
if nonce == nil {
Expand Down Expand Up @@ -900,7 +905,7 @@ func adaptDeclareTransaction(t *core.DeclareTransaction) *Transaction {
return tx
}

func adaptDeployAccountTrandaction(t *core.DeployAccountTransaction) *Transaction {
func adaptDeployAccountTransaction(t *core.DeployAccountTransaction) *Transaction {
tx := &Transaction{
Hash: t.Hash(),
MaxFee: t.MaxFee,
Expand Down
34 changes: 34 additions & 0 deletions rpc/v7/transaction_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1055,6 +1055,40 @@ func TestAddTransactionUnmarshal(t *testing.T) {
}
}

func TestAdaptTransaction(t *testing.T) {
t.Run("core.Resource `ResourceL1DataGas` should be ignored when converted to v7.Resource", func(t *testing.T) {
coreTx := core.InvokeTransaction{
Version: new(core.TransactionVersion).SetUint64(3),
ResourceBounds: map[core.Resource]core.ResourceBounds{
core.ResourceL1Gas: {MaxAmount: 1, MaxPricePerUnit: new(felt.Felt).SetUint64(2)},
core.ResourceL2Gas: {MaxAmount: 3, MaxPricePerUnit: new(felt.Felt).SetUint64(4)},
core.ResourceL1DataGas: {MaxAmount: 5, MaxPricePerUnit: new(felt.Felt).SetUint64(6)},
},
}

tx := rpc.AdaptTransaction(&coreTx)

expectedTx := &rpc.Transaction{
Type: rpc.TxnInvoke,
Version: new(felt.Felt).SetUint64(3),
ResourceBounds: &map[rpc.Resource]rpc.ResourceBounds{
rpc.ResourceL1Gas: {MaxAmount: new(felt.Felt).SetUint64(1), MaxPricePerUnit: new(felt.Felt).SetUint64(2)},
rpc.ResourceL2Gas: {MaxAmount: new(felt.Felt).SetUint64(3), MaxPricePerUnit: new(felt.Felt).SetUint64(4)},
},
Tip: new(felt.Felt).SetUint64(0),
// Those 4 fields are pointers to slice (the SliceHeader is allocated, it just refers to a nil array)
Signature: new([]*felt.Felt),
CallData: new([]*felt.Felt),
PaymasterData: new([]*felt.Felt),
AccountDeploymentData: new([]*felt.Felt),
NonceDAMode: utils.Ptr(rpc.DAModeL1),
FeeDAMode: utils.Ptr(rpc.DAModeL1),
}

require.Equal(t, expectedTx, tx)
})
}

func TestAddTransaction(t *testing.T) {
n := utils.Ptr(utils.Integration)
gw := adaptfeeder.New(feeder.NewTestClient(t, n))
Expand Down
90 changes: 44 additions & 46 deletions vm/rust/src/execution.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ pub fn is_l2_gas_accounting_enabled(
return Ok(false);
}


let min_sierra_version = &block_context
.versioned_constants()
.min_sierra_version_for_sierra_gas;
Expand Down Expand Up @@ -112,13 +111,13 @@ where

// Simulate transaction execution with maximum possible gas to get actual gas usage.
set_l2_gas_limit(transaction, GasAmount::MAX);
// TODO: Consider getting the upper bound from the balance and not changing the execution flags
// TODO: Consider getting the upper bound from the balance and not changing the execution flags
match transaction {
Transaction::Account(account_transaction) => {
account_transaction.execution_flags.charge_fee = false;
account_transaction.execution_flags.validate = false;
}
_ =>{},
_ => {}
};
let (simulation_result, _) = match simulate_execution(transaction, state, block_context) {
Ok(info) => info,
Expand All @@ -142,54 +141,53 @@ where
let l2_gas_adjusted = GasAmount(gas_used.saturating_add(gas_used / 10));
set_l2_gas_limit(transaction, l2_gas_adjusted);

let (l2_gas_limit, _, tx_state) =
match simulate_execution(transaction, state, block_context) {
Ok((tx_info, tx_state)) => {
// If 110% of the actual transaction gas fee is enough, we use that
// as the estimate and skip the binary search.
(l2_gas_adjusted, tx_info, tx_state)
}
Err(SimulationError::OutOfGas(_)) => {
let mut lower_bound = GasAmount(gas_used);
let mut upper_bound = GasAmount::MAX;
let mut current_l2_gas_limit = calculate_midpoint(lower_bound, upper_bound);

// Run a binary search to find the minimal gas limit that still allows the
// transaction to execute without running out of L2 gas.
let (tx_info, tx_state) = loop {
set_l2_gas_limit(transaction, current_l2_gas_limit);
let (l2_gas_limit, _, tx_state) = match simulate_execution(transaction, state, block_context) {
Ok((tx_info, tx_state)) => {
// If 110% of the actual transaction gas fee is enough, we use that
// as the estimate and skip the binary search.
(l2_gas_adjusted, tx_info, tx_state)
}
Err(SimulationError::OutOfGas(_)) => {
let mut lower_bound = GasAmount(gas_used);
let mut upper_bound = GasAmount::MAX;
let mut current_l2_gas_limit = calculate_midpoint(lower_bound, upper_bound);

// Special case where the search would get stuck if `current_l2_gas_limit ==
// lower_bound` but the required amount is equal to the upper bound.
let bounds_diff = upper_bound
.checked_sub(lower_bound)
.expect("Upper bound >= lower bound");
if bounds_diff == GasAmount(1) && current_l2_gas_limit == lower_bound {
lower_bound = upper_bound;
current_l2_gas_limit = upper_bound;
}
// Run a binary search to find the minimal gas limit that still allows the
// transaction to execute without running out of L2 gas.
let (tx_info, tx_state) = loop {
set_l2_gas_limit(transaction, current_l2_gas_limit);

match simulate_execution(transaction, state, block_context) {
Ok((tx_info, tx_state)) => {
if is_search_complete(lower_bound, upper_bound, L2_GAS_SEARCH_MARGIN) {
break (tx_info, tx_state);
}
// Special case where the search would get stuck if `current_l2_gas_limit ==
// lower_bound` but the required amount is equal to the upper bound.
let bounds_diff = upper_bound
.checked_sub(lower_bound)
.expect("Upper bound >= lower bound");
if bounds_diff == GasAmount(1) && current_l2_gas_limit == lower_bound {
lower_bound = upper_bound;
current_l2_gas_limit = upper_bound;
}

upper_bound = current_l2_gas_limit;
current_l2_gas_limit = calculate_midpoint(lower_bound, upper_bound);
}
Err(SimulationError::OutOfGas(_)) => {
lower_bound = current_l2_gas_limit;
current_l2_gas_limit = calculate_midpoint(lower_bound, upper_bound);
match simulate_execution(transaction, state, block_context) {
Ok((tx_info, tx_state)) => {
if is_search_complete(lower_bound, upper_bound, L2_GAS_SEARCH_MARGIN) {
break (tx_info, tx_state);
}
Err(SimulationError::ExecutionError(error)) => return Err(error),

upper_bound = current_l2_gas_limit;
current_l2_gas_limit = calculate_midpoint(lower_bound, upper_bound);
}
Err(SimulationError::OutOfGas(_)) => {
lower_bound = current_l2_gas_limit;
current_l2_gas_limit = calculate_midpoint(lower_bound, upper_bound);
}
};
Err(SimulationError::ExecutionError(error)) => return Err(error),
}
};

(current_l2_gas_limit, tx_info, tx_state)
}
Err(SimulationError::ExecutionError(error)) => return Err(error),
};
(current_l2_gas_limit, tx_info, tx_state)
}
Err(SimulationError::ExecutionError(error)) => return Err(error),
};
tx_state.abort();

// If the computed gas limit exceeds the initial limit, revert the transaction.
Expand All @@ -202,7 +200,7 @@ where

set_l2_gas_limit(&mut original_transaction, initial_gas_limit);
let mut simulated_state = CachedState::<_>::create_transactional(state);
let mut exec_info = original_transaction.execute(&mut simulated_state, block_context)?;
let mut exec_info = original_transaction.execute(&mut simulated_state, block_context)?;

// Execute the transaction with the determined gas limit and update the estimate.
simulated_state.commit();
Expand Down

0 comments on commit e354149

Please sign in to comment.