Skip to content

Commit

Permalink
feat: change the logic of gas cost calculation in silo mode (#854)
Browse files Browse the repository at this point in the history
## Description

The PR changes the logic of gas calculation in Silo mode. Fixed gas cost
has been replaced with a fixed amount of gas per transaction.

## Performance / NEAR gas cost considerations

There are no changes related to performance.

## Testing

Corresponding tests have been added.
  • Loading branch information
aleksuss authored Oct 23, 2023
1 parent ef93fcc commit c82d1b7
Show file tree
Hide file tree
Showing 19 changed files with 563 additions and 309 deletions.
11 changes: 9 additions & 2 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Changes

- Changed the logic of cost calculation in Silo mode. Specifically, the fixed cost has been replaced with
a fixed amount of gas per transaction by [@aleksuss]. ([#854])

[#854]: https://github.com/aurora-is-near/aurora-engine/pull/854

## [3.2.0] 2023-10-17

### Changes
Expand All @@ -18,7 +25,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Additions

- Added a possibility of mirroring deployed ERC-20 contracts in the main Aurora contract in Silo mode by [@aleksuss]. ([#844])
- Added a possibility of mirroring deployed ERC-20 contracts in the main Aurora contract in Silo mode by [@aleksuss]. ([#845])

- Allow to initialize hashchain directly with the `new` method by [@birchmd]. ([#846])

Expand All @@ -29,7 +36,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
[#746]: https://github.com/aurora-is-near/aurora-engine/pull/746
[#834]: https://github.com/aurora-is-near/aurora-engine/pull/834
[#840]: https://github.com/aurora-is-near/aurora-engine/pull/840
[#844]: https://github.com/aurora-is-near/aurora-engine/pull/844
[#845]: https://github.com/aurora-is-near/aurora-engine/pull/845
[#846]: https://github.com/aurora-is-near/aurora-engine/pull/846
[#847]: https://github.com/aurora-is-near/aurora-engine/pull/847

Expand Down
10 changes: 5 additions & 5 deletions engine-standalone-storage/src/sync/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -225,9 +225,9 @@ pub fn parse_transaction_kind(
})?;
TransactionKind::SetErc20Metadata(args)
}
TransactionKindTag::SetFixedGasCost => {
let args = silo_params::FixedGasCostArgs::try_from_slice(&bytes).map_err(f)?;
TransactionKind::SetFixedGasCost(args)
TransactionKindTag::SetFixedGas => {
let args = silo_params::FixedGasArgs::try_from_slice(&bytes).map_err(f)?;
TransactionKind::SetFixedGas(args)
}
TransactionKindTag::SetSiloParams => {
let args: Option<silo_params::SiloParamsArgs> =
Expand Down Expand Up @@ -691,8 +691,8 @@ fn non_submit_execute<I: IO + Copy>(

None
}
TransactionKind::SetFixedGasCost(args) => {
silo::set_fixed_gas_cost(&mut io, args.cost);
TransactionKind::SetFixedGas(args) => {
silo::set_fixed_gas(&mut io, args.fixed_gas);
None
}
TransactionKind::SetSiloParams(args) => {
Expand Down
20 changes: 9 additions & 11 deletions engine-standalone-storage/src/sync/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ pub enum TransactionKind {
/// Set metadata of ERC-20 contract.
SetErc20Metadata(parameters::SetErc20MetadataArgs),
/// Silo operations
SetFixedGasCost(silo::FixedGasCostArgs),
SetFixedGas(silo::FixedGasArgs),
SetSiloParams(Option<silo::SiloParamsArgs>),
AddEntryToWhitelist(silo::WhitelistArgs),
AddEntryToWhitelistBatch(Vec<silo::WhitelistArgs>),
Expand Down Expand Up @@ -401,7 +401,7 @@ impl TransactionKind {
Self::RemoveRelayerKey(_) => Self::no_evm_execution("remove_relayer_key"),
Self::StartHashchain(_) => Self::no_evm_execution("start_hashchain"),
Self::SetErc20Metadata(_) => Self::no_evm_execution("set_erc20_metadata"),
Self::SetFixedGasCost(_) => Self::no_evm_execution("set_fixed_gas_cost"),
Self::SetFixedGas(_) => Self::no_evm_execution("set_fixed_gas"),
Self::SetSiloParams(_) => Self::no_evm_execution("set_silo_params"),
Self::AddEntryToWhitelist(_) => Self::no_evm_execution("add_entry_to_whitelist"),
Self::AddEntryToWhitelistBatch(_) => {
Expand Down Expand Up @@ -528,8 +528,8 @@ pub enum TransactionKindTag {
SetErc20Metadata,
#[strum(serialize = "set_eth_connector_contract_account")]
SetEthConnectorContractAccount,
#[strum(serialize = "set_fixed_gas_cost")]
SetFixedGasCost,
#[strum(serialize = "set_fixed_gas")]
SetFixedGas,
#[strum(serialize = "set_silo_params")]
SetSiloParams,
#[strum(serialize = "set_whitelist_status")]
Expand Down Expand Up @@ -591,7 +591,7 @@ impl TransactionKind {
}
Self::StartHashchain(args) => args.try_to_vec().unwrap_or_default(),
Self::SetErc20Metadata(args) => serde_json::to_vec(args).unwrap_or_default(),
Self::SetFixedGasCost(args) => args.try_to_vec().unwrap_or_default(),
Self::SetFixedGas(args) => args.try_to_vec().unwrap_or_default(),
Self::SetSiloParams(args) => args.try_to_vec().unwrap_or_default(),
Self::AddEntryToWhitelist(args) | Self::RemoveEntryFromWhitelist(args) => {
args.try_to_vec().unwrap_or_default()
Expand Down Expand Up @@ -647,7 +647,7 @@ impl From<&TransactionKind> for TransactionKindTag {
TransactionKind::SetEthConnectorContractAccount(_) => {
Self::SetEthConnectorContractAccount
}
TransactionKind::SetFixedGasCost(_) => Self::SetFixedGasCost,
TransactionKind::SetFixedGas(_) => Self::SetFixedGas,
TransactionKind::SetSiloParams(_) => Self::SetSiloParams,
TransactionKind::AddEntryToWhitelist(_) => Self::AddEntryToWhitelist,
TransactionKind::AddEntryToWhitelistBatch(_) => Self::AddEntryToWhitelistBatch,
Expand Down Expand Up @@ -838,7 +838,7 @@ enum BorshableTransactionKind<'a> {
RemoveRelayerKey(Cow<'a, parameters::RelayerKeyArgs>),
StartHashchain(Cow<'a, parameters::StartHashchainArgs>),
SetErc20Metadata(Cow<'a, parameters::SetErc20MetadataArgs>),
SetFixedGasCost(Cow<'a, silo::FixedGasCostArgs>),
SetFixedGas(Cow<'a, silo::FixedGasArgs>),
SetSiloParams(Cow<'a, Option<silo::SiloParamsArgs>>),
AddEntryToWhitelist(Cow<'a, silo::WhitelistArgs>),
AddEntryToWhitelistBatch(Cow<'a, Vec<silo::WhitelistArgs>>),
Expand Down Expand Up @@ -902,7 +902,7 @@ impl<'a> From<&'a TransactionKind> for BorshableTransactionKind<'a> {
TransactionKind::RemoveRelayerKey(x) => Self::RemoveRelayerKey(Cow::Borrowed(x)),
TransactionKind::StartHashchain(x) => Self::StartHashchain(Cow::Borrowed(x)),
TransactionKind::SetErc20Metadata(x) => Self::SetErc20Metadata(Cow::Borrowed(x)),
TransactionKind::SetFixedGasCost(x) => Self::SetFixedGasCost(Cow::Borrowed(x)),
TransactionKind::SetFixedGas(x) => Self::SetFixedGas(Cow::Borrowed(x)),
TransactionKind::SetSiloParams(x) => Self::SetSiloParams(Cow::Borrowed(x)),
TransactionKind::AddEntryToWhitelist(x) => Self::AddEntryToWhitelist(Cow::Borrowed(x)),
TransactionKind::AddEntryToWhitelistBatch(x) => {
Expand Down Expand Up @@ -994,9 +994,7 @@ impl<'a> TryFrom<BorshableTransactionKind<'a>> for TransactionKind {
BorshableTransactionKind::SetErc20Metadata(x) => {
Ok(Self::SetErc20Metadata(x.into_owned()))
}
BorshableTransactionKind::SetFixedGasCost(x) => {
Ok(Self::SetFixedGasCost(x.into_owned()))
}
BorshableTransactionKind::SetFixedGas(x) => Ok(Self::SetFixedGas(x.into_owned())),
BorshableTransactionKind::SetSiloParams(x) => Ok(Self::SetSiloParams(x.into_owned())),
BorshableTransactionKind::AddEntryToWhitelist(x) => {
Ok(Self::AddEntryToWhitelist(x.into_owned()))
Expand Down
9 changes: 6 additions & 3 deletions engine-tests/src/tests/erc20.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,15 +86,17 @@ fn erc20_mint_out_of_gas() {
utils::address_from_secret_key(&source_account.secret_key),
Wei::new_u64(INITIAL_BALANCE - GAS_LIMIT * GAS_PRICE),
(INITIAL_NONCE + 2).into(),
);
)
.unwrap();
utils::validate_address_balance_and_nonce(
&runner,
sdk::types::near_account_to_evm_address(
runner.context.predecessor_account_id.as_ref().as_bytes(),
),
Wei::new_u64(GAS_LIMIT * GAS_PRICE),
U256::zero(),
);
)
.unwrap();
}

#[test]
Expand Down Expand Up @@ -239,7 +241,8 @@ fn deploy_erc_20_out_of_gas() {
utils::address_from_secret_key(&source_account),
Wei::new_u64(INITIAL_BALANCE),
(INITIAL_NONCE + 1).into(),
);
)
.unwrap();
}

#[test]
Expand Down
89 changes: 59 additions & 30 deletions engine-tests/src/tests/sanity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -707,8 +707,10 @@ fn test_eth_transfer_success() {
source_address,
INITIAL_BALANCE,
INITIAL_NONCE.into(),
);
utils::validate_address_balance_and_nonce(&runner, dest_address, Wei::zero(), 0.into());
)
.unwrap();
utils::validate_address_balance_and_nonce(&runner, dest_address, Wei::zero(), 0.into())
.unwrap();

// perform transfer
runner
Expand All @@ -723,8 +725,10 @@ fn test_eth_transfer_success() {
source_address,
INITIAL_BALANCE - TRANSFER_AMOUNT,
(INITIAL_NONCE + 1).into(),
);
utils::validate_address_balance_and_nonce(&runner, dest_address, TRANSFER_AMOUNT, 0.into());
)
.unwrap();
utils::validate_address_balance_and_nonce(&runner, dest_address, TRANSFER_AMOUNT, 0.into())
.unwrap();
}

/// Tests the case where the transfer amount is larger than the address balance
Expand All @@ -739,8 +743,10 @@ fn test_eth_transfer_insufficient_balance() {
source_address,
INITIAL_BALANCE,
INITIAL_NONCE.into(),
);
utils::validate_address_balance_and_nonce(&runner, dest_address, Wei::zero(), 0.into());
)
.unwrap();
utils::validate_address_balance_and_nonce(&runner, dest_address, Wei::zero(), 0.into())
.unwrap();

// attempt transfer
let result = runner
Expand All @@ -758,8 +764,10 @@ fn test_eth_transfer_insufficient_balance() {
INITIAL_BALANCE,
// the nonce is still incremented even though the transfer failed
(INITIAL_NONCE + 1).into(),
);
utils::validate_address_balance_and_nonce(&runner, dest_address, Wei::zero(), 0.into());
)
.unwrap();
utils::validate_address_balance_and_nonce(&runner, dest_address, Wei::zero(), 0.into())
.unwrap();
}

/// Tests the case where the nonce on the transaction does not match the address
Expand All @@ -774,8 +782,10 @@ fn test_eth_transfer_incorrect_nonce() {
source_address,
INITIAL_BALANCE,
INITIAL_NONCE.into(),
);
utils::validate_address_balance_and_nonce(&runner, dest_address, Wei::zero(), 0.into());
)
.unwrap();
utils::validate_address_balance_and_nonce(&runner, dest_address, Wei::zero(), 0.into())
.unwrap();

// attempt transfer
let error = runner
Expand All @@ -792,8 +802,10 @@ fn test_eth_transfer_incorrect_nonce() {
source_address,
INITIAL_BALANCE,
INITIAL_NONCE.into(),
);
utils::validate_address_balance_and_nonce(&runner, dest_address, Wei::zero(), 0.into());
)
.unwrap();
utils::validate_address_balance_and_nonce(&runner, dest_address, Wei::zero(), 0.into())
.unwrap();
}

#[test]
Expand Down Expand Up @@ -837,8 +849,10 @@ fn test_eth_transfer_not_enough_gas() {
source_address,
INITIAL_BALANCE,
INITIAL_NONCE.into(),
);
utils::validate_address_balance_and_nonce(&runner, dest_address, Wei::zero(), 0.into());
)
.unwrap();
utils::validate_address_balance_and_nonce(&runner, dest_address, Wei::zero(), 0.into())
.unwrap();

// attempt transfer
let error = runner
Expand All @@ -852,8 +866,10 @@ fn test_eth_transfer_not_enough_gas() {
source_address,
INITIAL_BALANCE,
INITIAL_NONCE.into(),
);
utils::validate_address_balance_and_nonce(&runner, dest_address, Wei::zero(), 0.into());
)
.unwrap();
utils::validate_address_balance_and_nonce(&runner, dest_address, Wei::zero(), 0.into())
.unwrap();
}

#[test]
Expand All @@ -873,8 +889,10 @@ fn test_transfer_charging_gas_success() {
source_address,
INITIAL_BALANCE,
INITIAL_NONCE.into(),
);
utils::validate_address_balance_and_nonce(&runner, dest_address, Wei::zero(), 0.into());
)
.unwrap();
utils::validate_address_balance_and_nonce(&runner, dest_address, Wei::zero(), 0.into())
.unwrap();

// do transfer
let result = runner
Expand All @@ -894,19 +912,22 @@ fn test_transfer_charging_gas_success() {
source_address,
expected_source_balance,
(INITIAL_NONCE + 1).into(),
);
)
.unwrap();
utils::validate_address_balance_and_nonce(
&runner,
dest_address,
expected_dest_balance,
0.into(),
);
)
.unwrap();
utils::validate_address_balance_and_nonce(
&runner,
relayer_address,
expected_relayer_balance,
0.into(),
);
)
.unwrap();
}

#[test]
Expand All @@ -928,8 +949,10 @@ fn test_eth_transfer_charging_gas_not_enough_balance() {
source_address,
INITIAL_BALANCE,
INITIAL_NONCE.into(),
);
utils::validate_address_balance_and_nonce(&runner, dest_address, Wei::zero(), 0.into());
)
.unwrap();
utils::validate_address_balance_and_nonce(&runner, dest_address, Wei::zero(), 0.into())
.unwrap();

// attempt transfer
let error = runner
Expand All @@ -951,9 +974,11 @@ fn test_eth_transfer_charging_gas_not_enough_balance() {
INITIAL_BALANCE,
// nonce is still not incremented since the transaction was invalid
INITIAL_NONCE.into(),
);
utils::validate_address_balance_and_nonce(&runner, dest_address, Wei::zero(), 0.into());
utils::validate_address_balance_and_nonce(&runner, relayer, Wei::zero(), 0.into());
)
.unwrap();
utils::validate_address_balance_and_nonce(&runner, dest_address, Wei::zero(), 0.into())
.unwrap();
utils::validate_address_balance_and_nonce(&runner, relayer, Wei::zero(), 0.into()).unwrap();
}

pub fn initialize_transfer() -> (utils::AuroraRunner, utils::Signer, Address) {
Expand Down Expand Up @@ -1071,8 +1096,10 @@ fn test_eth_transfer_with_max_gas_price() {
source_address,
INITIAL_BALANCE,
INITIAL_NONCE.into(),
);
utils::validate_address_balance_and_nonce(&runner, dest_address, Wei::zero(), 0.into());
)
.unwrap();
utils::validate_address_balance_and_nonce(&runner, dest_address, Wei::zero(), 0.into())
.unwrap();

// perform transfer
let max_gas_price = 5;
Expand All @@ -1091,8 +1118,10 @@ fn test_eth_transfer_with_max_gas_price() {
source_address,
INITIAL_BALANCE - TRANSFER_AMOUNT - Wei::new_u128(fee),
(INITIAL_NONCE + 1).into(),
);
utils::validate_address_balance_and_nonce(&runner, dest_address, TRANSFER_AMOUNT, 0.into());
)
.unwrap();
utils::validate_address_balance_and_nonce(&runner, dest_address, TRANSFER_AMOUNT, 0.into())
.unwrap();
}

#[test]
Expand Down
Loading

0 comments on commit c82d1b7

Please sign in to comment.