Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

auto: fix chain module #14140

Closed
wants to merge 6 commits into from
Closed

auto: fix chain module #14140

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0545615
auto: fix chain module
FelixFan1992 Aug 16, 2024
03ad8f6
Merge 05456152841b3350f528a5dfa0240bbcb83b9f30 into e780e5c1972ae0912…
FelixFan1992 Aug 16, 2024
1594fb6
Update gethwrappers
app-token-issuer-infra-releng[bot] Aug 16, 2024
d86b5a3
update
FelixFan1992 Aug 16, 2024
bf41b09
add scroll module V2
FelixFan1992 Aug 19, 2024
873887c
update
FelixFan1992 Aug 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion contracts/src/v0.8/automation/chains/ArbitrumModule.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ contract ArbitrumModule is ChainModuleBase {
return ARB_SYS.arbBlockNumber();
}

function getCurrentL1Fee() external view override returns (uint256) {
function getCurrentL1Fee(uint256) external view override returns (uint256) {
return ARB_GAS.getCurrentTxL1GasFees();
}

Expand Down
2 changes: 1 addition & 1 deletion contracts/src/v0.8/automation/chains/ChainModuleBase.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ contract ChainModuleBase is IChainModule {
return blockhash(n);
}

function getCurrentL1Fee() external view virtual returns (uint256) {
function getCurrentL1Fee(uint256) external view virtual returns (uint256) {
return 0;
}

Expand Down
2 changes: 1 addition & 1 deletion contracts/src/v0.8/automation/chains/OptimismModule.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ contract OptimismModule is ChainModuleBase {
uint256 private constant FIXED_GAS_OVERHEAD = 60_000;
uint256 private constant PER_CALLDATA_BYTE_GAS_OVERHEAD = 270;

function getCurrentL1Fee() external view override returns (uint256) {
function getCurrentL1Fee(uint256) external view override returns (uint256) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

using uint256 will make it not accurate
using bytes will make it waste lots of gas

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. passing bytes into this function
  2. concat this bytes with a padding
  3. passing the concated bytes to getL1Fee

with chain module (uint256) and V2: nothing
with chain module (bytes) and V2: 1 (BASE, OP), 1/2/3 (Scroll)

return OVM_GASPRICEORACLE.getL1Fee(bytes.concat(msg.data, OP_L1_DATA_FEE_PADDING));
}

Expand Down
4 changes: 2 additions & 2 deletions contracts/src/v0.8/automation/chains/OptimismModuleV2.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ contract OptimismModuleV2 is ChainModuleBase, ConfirmedOwner {

constructor() ConfirmedOwner(msg.sender) {}

function getCurrentL1Fee() external view override returns (uint256) {
return (s_l1FeeCoefficient * _getL1Fee(msg.data.length)) / 100;
function getCurrentL1Fee(uint256 dataSize) external view override returns (uint256) {
return (s_l1FeeCoefficient * _getL1Fee(dataSize)) / 100;
}

function getMaxL1Fee(uint256 dataSize) external view override returns (uint256) {
Expand Down
2 changes: 1 addition & 1 deletion contracts/src/v0.8/automation/chains/ScrollModule.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ contract ScrollModule is ChainModuleBase {
uint256 private constant FIXED_GAS_OVERHEAD = 45_000;
uint256 private constant PER_CALLDATA_BYTE_GAS_OVERHEAD = 170;

function getCurrentL1Fee() external view override returns (uint256) {
function getCurrentL1Fee(uint256) external view override returns (uint256) {
return SCROLL_ORACLE.getL1Fee(bytes.concat(msg.data, SCROLL_L1_FEE_DATA_PADDING));
}

Expand Down
2 changes: 1 addition & 1 deletion contracts/src/v0.8/automation/interfaces/IChainModule.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ interface IChainModule {
// retrieve the L1 data fee for a L2 transaction. it should return 0 for L1 chains and
// L2 chains which don't have L1 fee component. it uses msg.data to estimate L1 data so
// it must be used with a transaction. Return value in wei.
function getCurrentL1Fee() external view returns (uint256);
function getCurrentL1Fee(uint256 dataSize) external view returns (uint256);
Copy link
Contributor Author

@FelixFan1992 FelixFan1992 Aug 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

option 2 is use a bytes here. but that makes L1 fee calculation extremely costly and that's what we try to avoid


// retrieve the L1 data fee for a L2 simulation. it should return 0 for L1 chains and
// L2 chains which don't have L1 fee component. Return value in wei.
Expand Down
2 changes: 1 addition & 1 deletion contracts/src/v0.8/automation/v2_2/AutomationRegistry2_2.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ contract AutomationRegistry2_2 is AutomationRegistryBase2_2, OCR2Abstract, Chain
});

uint256 blocknumber = hotVars.chainModule.blockNumber();
uint256 l1Fee = hotVars.chainModule.getCurrentL1Fee();
uint256 l1Fee = hotVars.chainModule.getCurrentL1Fee(msg.data.length);

for (uint256 i = 0; i < report.upkeepIds.length; i++) {
upkeepTransmitInfo[i].upkeep = s_upkeep[report.upkeepIds[i]];
Expand Down
2 changes: 1 addition & 1 deletion contracts/src/v0.8/automation/v2_3/AutomationRegistry2_3.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ contract AutomationRegistry2_3 is AutomationRegistryBase2_3, OCR2Abstract, Chain
});

uint256 blocknumber = hotVars.chainModule.blockNumber();
uint256 l1Fee = hotVars.chainModule.getCurrentL1Fee();
uint256 l1Fee = hotVars.chainModule.getCurrentL1Fee(msg.data.length);

for (uint256 i = 0; i < report.upkeepIds.length; i++) {
upkeepTransmitInfo[i].upkeep = s_upkeep[report.upkeepIds[i]];
Expand Down
18 changes: 9 additions & 9 deletions core/gethwrappers/generated/arbitrum_module/arbitrum_module.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion ...gethwrappers/generated/automation_registry_wrapper_2_2/automation_registry_wrapper_2_2.go
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion ...gethwrappers/generated/automation_registry_wrapper_2_3/automation_registry_wrapper_2_3.go
View file
Open in desktop

Large diffs are not rendered by default.

18 changes: 9 additions & 9 deletions core/gethwrappers/generated/chain_module_base/chain_module_base.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading