Skip to content

Commit

Permalink
refactor(contracts/core): improved variable packing to reduce gas cos…
Browse files Browse the repository at this point in the history 
…ts (#2291)

Optimized all variables to reduce contract storage slots from 4 to 2, and the struct's required SLOAD/SREADs from 5 to 2. This leads to a ~45% gas reduction in the bulkSetFeeParams() function and ~8% in the user-facing feeFor() function.

issue: #1951
  • Loading branch information
@Zodomo
Zodomo authored Oct 28, 2024
1 parent 4e94882 commit a80ae44
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 62 deletions.
30 changes: 15 additions & 15 deletions contracts/core/.gas-snapshot
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,21 +7,21 @@ AllocPredeploys_Test:test_num_allocs() (gas: 1181198035)
AllocPredeploys_Test:test_predeploys() (gas: 1181134337)
AllocPredeploys_Test:test_preinstalls() (gas: 1181850775)
AllocPredeploys_Test:test_proxies() (gas: 1408777576)
FeeOracleV1_Test:test_bulkSetFeeParams() (gas: 173109)
FeeOracleV1_Test:test_feeFor() (gas: 122852)
FeeOracleV1_Test:test_setBaseGasLimit() (gas: 32397)
FeeOracleV1_Test:test_setGasPrice() (gas: 41034)
FeeOracleV1_Test:test_setManager() (gas: 45904)
FeeOracleV1_Test:test_setProtocolFee() (gas: 31610)
FeeOracleV1_Test:test_setToNativeRate() (gas: 41132)
FeeOracleV2_Test:test_bulkSetFeeParams() (gas: 218007)
FeeOracleV2_Test:test_feeFor() (gas: 111934)
FeeOracleV2_Test:test_setBaseGasLimit() (gas: 32261)
FeeOracleV2_Test:test_setDataGasPrice() (gas: 43312)
FeeOracleV2_Test:test_setExecGasPrice() (gas: 43184)
FeeOracleV2_Test:test_setManager() (gas: 45827)
FeeOracleV2_Test:test_setProtocolFee() (gas: 31508)
FeeOracleV2_Test:test_setToNativeRate() (gas: 43377)
FeeOracleV1_Test:test_bulkSetFeeParams() (gas: 172862)
FeeOracleV1_Test:test_feeFor() (gas: 122551)
FeeOracleV1_Test:test_setBaseGasLimit() (gas: 32208)
FeeOracleV1_Test:test_setGasPrice() (gas: 40996)
FeeOracleV1_Test:test_setManager() (gas: 45845)
FeeOracleV1_Test:test_setProtocolFee() (gas: 31442)
FeeOracleV1_Test:test_setToNativeRate() (gas: 41049)
FeeOracleV2_Test:test_bulkSetFeeParams() (gas: 119117)
FeeOracleV2_Test:test_feeFor() (gas: 103301)
FeeOracleV2_Test:test_setBaseGasLimit() (gas: 32009)
FeeOracleV2_Test:test_setDataGasPrice() (gas: 44216)
FeeOracleV2_Test:test_setExecGasPrice() (gas: 44247)
FeeOracleV2_Test:test_setManager() (gas: 45775)
FeeOracleV2_Test:test_setProtocolFee() (gas: 32226)
FeeOracleV2_Test:test_setToNativeRate() (gas: 43640)
InitializableHelper_Test:test_disableInitalizers() (gas: 181686)
InitializableHelper_Test:test_getInitialized() (gas: 178023)
OmniBridgeL1_Test:test_bridge() (gas: 233678)
Expand Down
36 changes: 18 additions & 18 deletions contracts/core/src/interfaces/IFeeOracleV2.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,22 +10,22 @@ import { IConversionRateOracle } from "./IConversionRateOracle.sol";
*/
interface IFeeOracleV2 is IFeeOracle, IConversionRateOracle {
/// @notice Emitted when fee parameters for a chain are set.
event FeeParamsSet(uint64 chainId, uint256 execGasPrice, uint256 dataGasPrice, uint256 toNativeRate);
event FeeParamsSet(uint64 chainId, uint64 execGasPrice, uint64 dataGasPrice, uint64 toNativeRate);

/// @notice Emitted when the base gas limit is set.
event BaseGasLimitSet(uint64 baseGasLimit);
event BaseGasLimitSet(uint24 baseGasLimit);

/// @notice Emitted when the base protocol fee is set.
event ProtocolFeeSet(uint256 protocolFee);
event ProtocolFeeSet(uint72 protocolFee);

/// @notice Emitted when the gas price for a destination chain is set.
event ExecGasPriceSet(uint64 chainId, uint256 gasPrice);
event ExecGasPriceSet(uint64 chainId, uint64 gasPrice);

/// @notice Emitted when the data gas price for a destination chain is set.
event DataGasPriceSet(uint64 chainId, uint256 gasPrice);
event DataGasPriceSet(uint64 chainId, uint64 gasPrice);

/// @notice Emitted when the to-native conversion rate for a destination chain is set.
event ToNativeRateSet(uint64 chainId, uint256 toNativeRate);
event ToNativeRateSet(uint64 chainId, uint64 toNativeRate);

/// @notice Emitted when the manager is changed.
event ManagerSet(address manager);
Expand All @@ -41,19 +41,19 @@ interface IFeeOracleV2 is IFeeOracle, IConversionRateOracle {
*/
struct FeeParams {
uint64 chainId;
uint256 execGasPrice;
uint256 dataGasPrice;
uint256 toNativeRate;
uint64 execGasPrice;
uint64 dataGasPrice;
uint64 toNativeRate;
}

/// @notice Returns the fee parameters for a destination chain.
function feeParams(uint64 chainId) external view returns (FeeParams memory);

/// @notice Returns the execution gas price for a destination chain.
function execGasPrice(uint64 chainId) external view returns (uint256);
function execGasPrice(uint64 chainId) external view returns (uint64);

/// @notice Returns the data gas price for a destination chain.
function dataGasPrice(uint64 chainId) external view returns (uint256);
function dataGasPrice(uint64 chainId) external view returns (uint64);

/// @notice Returns the to-native conversion rate for a destination chain.
function toNativeRate(uint64 chainId) external view returns (uint256);
Expand All @@ -62,28 +62,28 @@ interface IFeeOracleV2 is IFeeOracle, IConversionRateOracle {
function manager() external view returns (address);

/// @notice Returns the protocol fee.
function protocolFee() external view returns (uint256);
function protocolFee() external view returns (uint72);

/// @notice Returns the base gas limit.
function baseGasLimit() external view returns (uint64);
function baseGasLimit() external view returns (uint24);

/// @notice Set the fee parameters for a list of destination chains.
function bulkSetFeeParams(FeeParams[] calldata params) external;

/// @notice Set the execution gas price for a destination chain.
function setExecGasPrice(uint64 chainId, uint256 execGasPrice) external;
function setExecGasPrice(uint64 chainId, uint64 execGasPrice) external;

/// @notice Set the data gas price for a destination chain.
function setDataGasPrice(uint64 chainId, uint256 dataGasPrice) external;
function setDataGasPrice(uint64 chainId, uint64 dataGasPrice) external;

/// @notice Set the to native conversion rate for a destination chain.
function setToNativeRate(uint64 chainId, uint256 toNativeRate) external;
function setToNativeRate(uint64 chainId, uint64 toNativeRate) external;

/// @notice Set the base gas limit for each xmsg.
function setBaseGasLimit(uint64 gasLimit) external;
function setBaseGasLimit(uint24 gasLimit) external;

/// @notice Set the base protocol fee for each xmsg.
function setProtocolFee(uint256 fee) external;
function setProtocolFee(uint72 fee) external;

/// @notice Set the manager admin account.
function setManager(address manager) external;
Expand Down
36 changes: 18 additions & 18 deletions contracts/core/src/xchain/FeeOracleV2.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,14 @@ import { IFeeOracleV2 } from "../interfaces/IFeeOracleV2.sol";
*/
contract FeeOracleV2 is IFeeOracle, IFeeOracleV2, OwnableUpgradeable {
/**
* @notice Base gas limit for each xmsg.
* @notice Base protocol fee for each xmsg.
*/
uint64 public baseGasLimit;
uint72 public protocolFee;

/**
* @notice Base protocol fee for each xmsg.
* @notice Base gas limit for each xmsg.
*/
uint256 public protocolFee;
uint24 public baseGasLimit;

/**
* @notice Address allowed to set gas prices and to-native conversion rates.
Expand Down Expand Up @@ -48,8 +48,8 @@ contract FeeOracleV2 is IFeeOracle, IFeeOracleV2, OwnableUpgradeable {
function initialize(
address owner_,
address manager_,
uint64 baseGasLimit_,
uint256 protocolFee_,
uint24 baseGasLimit_,
uint72 protocolFee_,
FeeParams[] calldata params
) public initializer {
__Ownable_init(owner_);
Expand Down Expand Up @@ -93,14 +93,14 @@ contract FeeOracleV2 is IFeeOracle, IFeeOracleV2, OwnableUpgradeable {
/**
* @notice Returns the gas price for a destination chain.
*/
function execGasPrice(uint64 chainId) external view returns (uint256) {
function execGasPrice(uint64 chainId) external view returns (uint64) {
return _feeParams[chainId].execGasPrice;
}

/**
* @notice Returns the data gas price for a destination chain.
*/
function dataGasPrice(uint64 chainId) external view returns (uint256) {
function dataGasPrice(uint64 chainId) external view returns (uint64) {
return _feeParams[chainId].dataGasPrice;
}

Expand All @@ -121,35 +121,35 @@ contract FeeOracleV2 is IFeeOracle, IFeeOracleV2, OwnableUpgradeable {
/**
* @notice Set the execution gas price for a destination chain.
*/
function setExecGasPrice(uint64 chainId, uint256 gasPrice) external onlyManager {
function setExecGasPrice(uint64 chainId, uint64 gasPrice) external onlyManager {
_setExecGasPrice(chainId, gasPrice);
}

/**
* @notice Set the data gas price for a destination chain.
*/
function setDataGasPrice(uint64 chainId, uint256 gasPrice) external onlyManager {
function setDataGasPrice(uint64 chainId, uint64 gasPrice) external onlyManager {
_setDataGasPrice(chainId, gasPrice);
}

/**
* @notice Set the to native conversion rate for a destination chain.
*/
function setToNativeRate(uint64 chainId, uint256 rate) external onlyManager {
function setToNativeRate(uint64 chainId, uint64 rate) external onlyManager {
_setToNativeRate(chainId, rate);
}

/**
* @notice Set the base gas limit for each xmsg.
*/
function setBaseGasLimit(uint64 gasLimit) external onlyOwner {
function setBaseGasLimit(uint24 gasLimit) external onlyOwner {
_setBaseGasLimit(gasLimit);
}

/**
* @notice Set the base protocol fee for each xmsg.
*/
function setProtocolFee(uint256 fee) external onlyOwner {
function setProtocolFee(uint72 fee) external onlyOwner {
_setProtocolFee(fee);
}

Expand Down Expand Up @@ -182,7 +182,7 @@ contract FeeOracleV2 is IFeeOracle, IFeeOracleV2, OwnableUpgradeable {
/**
* @notice Set the execution gas price for a destination chain.
*/
function _setExecGasPrice(uint64 chainId, uint256 gasPrice) internal {
function _setExecGasPrice(uint64 chainId, uint64 gasPrice) internal {
require(gasPrice > 0, "FeeOracleV2: no zero gas price");
require(chainId != 0, "FeeOracleV2: no zero chain id");

Expand All @@ -193,7 +193,7 @@ contract FeeOracleV2 is IFeeOracle, IFeeOracleV2, OwnableUpgradeable {
/**
* @notice Set the data gas price for a destination chain.
*/
function _setDataGasPrice(uint64 chainId, uint256 gasPrice) internal {
function _setDataGasPrice(uint64 chainId, uint64 gasPrice) internal {
require(gasPrice > 0, "FeeOracleV2: no zero gas price");
require(chainId != 0, "FeeOracleV2: no zero chain id");

Expand All @@ -204,7 +204,7 @@ contract FeeOracleV2 is IFeeOracle, IFeeOracleV2, OwnableUpgradeable {
/**
* @notice Set the to-native conversion rate for a destination chain.
*/
function _setToNativeRate(uint64 chainId, uint256 rate) internal {
function _setToNativeRate(uint64 chainId, uint64 rate) internal {
require(rate > 0, "FeeOracleV2: no zero rate");
require(chainId != 0, "FeeOracleV2: no zero chain id");

Expand All @@ -215,15 +215,15 @@ contract FeeOracleV2 is IFeeOracle, IFeeOracleV2, OwnableUpgradeable {
/**
* @notice Set the base gas limit for each xmsg.
*/
function _setBaseGasLimit(uint64 gasLimit) internal {
function _setBaseGasLimit(uint24 gasLimit) internal {
baseGasLimit = gasLimit;
emit BaseGasLimitSet(gasLimit);
}

/**
* @notice Set the base protocol fee for each xmsg.
*/
function _setProtocolFee(uint256 fee) internal {
function _setProtocolFee(uint72 fee) internal {
protocolFee = fee;
emit ProtocolFeeSet(fee);
}
Expand Down
22 changes: 11 additions & 11 deletions contracts/core/test/xchain/FeeOracleV2.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ contract FeeOracleV2_Test is Test {
assertEq(fee, feeOracle.feeFor(destChainId, data, gasLimit)); // should be stable

// change exec gas price
uint256 gasPrice = feeOracle.execGasPrice(destChainId);
uint64 gasPrice = feeOracle.execGasPrice(destChainId);

feeOracle.setExecGasPrice(destChainId, gasPrice * 2);
assertEq(fee < feeOracle.feeFor(destChainId, data, gasLimit), true); // should be higher now
Expand All @@ -76,7 +76,7 @@ contract FeeOracleV2_Test is Test {
feeOracle.setExecGasPrice(destChainId, gasPrice); // reset

// change to native rate
uint256 toNativeRate = feeOracle.toNativeRate(destChainId);
uint64 toNativeRate = uint64(feeOracle.toNativeRate(destChainId));

feeOracle.setToNativeRate(destChainId, toNativeRate * 2);
assertEq(fee < feeOracle.feeFor(destChainId, data, gasLimit), true); // should be higher
Expand All @@ -88,7 +88,7 @@ contract FeeOracleV2_Test is Test {

// change data gas price

uint256 dataGasPrice = feeOracle.dataGasPrice(destChainId);
uint64 dataGasPrice = feeOracle.dataGasPrice(destChainId);

feeOracle.setDataGasPrice(destChainId, dataGasPrice * 2);
assertEq(fee < feeOracle.feeFor(destChainId, data, gasLimit), true); // should be higher
Expand Down Expand Up @@ -129,7 +129,7 @@ contract FeeOracleV2_Test is Test {

function test_setExecGasPrice() public {
uint64 destChainId = chainAId;
uint256 newGasPrice = feeOracle.execGasPrice(destChainId) + 1 gwei;
uint64 newGasPrice = feeOracle.execGasPrice(destChainId) + 1 gwei;

// only manager can set gas price
vm.expectRevert("FeeOracleV2: not manager");
Expand All @@ -153,7 +153,7 @@ contract FeeOracleV2_Test is Test {

function test_setDataGasPrice() public {
uint64 destChainId = chainAId;
uint256 newGasPrice = feeOracle.dataGasPrice(destChainId) + 1 gwei;
uint64 newGasPrice = feeOracle.dataGasPrice(destChainId) + 1 gwei;

// only manager can set gas price
vm.expectRevert("FeeOracleV2: not manager");
Expand All @@ -176,7 +176,7 @@ contract FeeOracleV2_Test is Test {
}

function test_setProtocolFee() public {
uint256 newProtocolFee = feeOracle.protocolFee() + 1 gwei;
uint72 newProtocolFee = feeOracle.protocolFee() + 1 gwei;

// only owner can set protocol fee
address notOwner = address(0x456);
Expand All @@ -191,7 +191,7 @@ contract FeeOracleV2_Test is Test {
}

function test_setBaseGasLimit() public {
uint64 newBaseGasLimit = feeOracle.baseGasLimit() + 10_000;
uint24 newBaseGasLimit = feeOracle.baseGasLimit() + 10_000;

// only owner can set base gas limit
address notOwner = address(0x456);
Expand All @@ -207,7 +207,7 @@ contract FeeOracleV2_Test is Test {

function test_setToNativeRate() public {
uint64 destChainId = chainAId;
uint256 newToNativeRate = feeOracle.toNativeRate(destChainId) + 1;
uint64 newToNativeRate = uint64(feeOracle.toNativeRate(destChainId) + 1);

// only manager can set to native rate
vm.expectRevert("FeeOracleV2: not manager");
Expand Down Expand Up @@ -236,21 +236,21 @@ contract FeeOracleV2_Test is Test {
chainId: chainAId,
execGasPrice: feeOracle.execGasPrice(chainAId) + 1 gwei,
dataGasPrice: feeOracle.dataGasPrice(chainAId) + 2 gwei,
toNativeRate: feeOracle.toNativeRate(chainAId) + 1
toNativeRate: uint64(feeOracle.toNativeRate(chainAId) + 1)
});

feeParams[1] = IFeeOracleV2.FeeParams({
chainId: chainBId,
execGasPrice: feeOracle.execGasPrice(chainBId) + 2 gwei,
dataGasPrice: feeOracle.dataGasPrice(chainBId) + 3 gwei,
toNativeRate: feeOracle.toNativeRate(chainBId) + 2
toNativeRate: uint64(feeOracle.toNativeRate(chainBId) + 2)
});

feeParams[2] = IFeeOracleV2.FeeParams({
chainId: chainCId,
execGasPrice: feeOracle.execGasPrice(chainCId) + 3 gwei,
dataGasPrice: feeOracle.dataGasPrice(chainCId) + 4 gwei,
toNativeRate: feeOracle.toNativeRate(chainCId) + 3
toNativeRate: uint64(feeOracle.toNativeRate(chainCId) + 3)
});

feeParams[3] = IFeeOracleV2.FeeParams({
Expand Down

0 comments on commit a80ae44

Please sign in to comment.