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.

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

Update L2 fees logic #808

Merged
merged 2 commits into from
Sep 25, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
14 changes: 14 additions & 0 deletions examples/fee-tester/.eslintrc.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
module.exports = {
extends: ["plugin:prettier/recommended"],
parserOptions: {
ecmaVersion: "latest",
},
env: {
es6: true,
node: true,
},
rules: {
"no-console": "error",
},
ignorePatterns: [".eslintrc.js", "artifacts/*", "cache/*"],
};
13 changes: 13 additions & 0 deletions examples/fee-tester/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
node_modules
.env
coverage
coverage.json
typechain
typechain-types

#Hardhat files
cache
artifacts

ignition/deployments

5 changes: 5 additions & 0 deletions examples/fee-tester/.prettierignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
/node_modules
/artifacts
/cache
/coverage
/.nyc_output
1 change: 1 addition & 0 deletions examples/fee-tester/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# Hardhat Fee Tester for Hardhat Ignition
16 changes: 16 additions & 0 deletions examples/fee-tester/contracts/FeeTester.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.9;


contract FeeTester {
address public owner;

constructor() {
owner = msg.sender;
}

// arbitrary function to call during fee testing
function deleteOwner() public {
delete owner;
}
}
7 changes: 7 additions & 0 deletions examples/fee-tester/hardhat.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
require("@nomicfoundation/hardhat-toolbox");
require("@nomicfoundation/hardhat-ignition-ethers");

/** @type import('hardhat/config').HardhatUserConfig */
module.exports = {
solidity: "0.8.19",
};
12 changes: 12 additions & 0 deletions examples/fee-tester/ignition/modules/FeeTesterModule.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// ./ignition/FeeTesterModule.js
const { buildModule } = require("@nomicfoundation/hardhat-ignition/modules");

module.exports = buildModule("FeeTesterModule", (m) => {
const feeTester1 = m.contract("FeeTester");

m.call(feeTester1, "deleteOwner");

const feeTester2 = m.contract("FeeTester", [], { id: "feeTester2" });

return { feeTester1, feeTester2 };
});
18 changes: 18 additions & 0 deletions examples/fee-tester/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
{
"name": "@nomicfoundation/ignition-fee-tester-example",
"private": true,
"version": "0.15.2",
"scripts": {
"test": "hardhat test",
"lint": "npm run prettier -- --check && npm run eslint",
"lint:fix": "npm run prettier -- --write && npm run eslint -- --fix",
"eslint": "eslint \"ignition/**/*.{js,jsx}\" \"test/**/*.{js,jsx}\"",
"prettier": "prettier \"*.{js,md,json}\" \"ignition/modules/*.{js,md,json}\" \"test/*.{js,md,json}\" \"contracts/**/*.sol\""
},
"devDependencies": {
"@nomicfoundation/hardhat-ignition-ethers": "workspace:^",
"@nomicfoundation/hardhat-toolbox": "4.0.0",
"hardhat": "^2.18.0",
"prettier-plugin-solidity": "1.1.3"
}
}
6 changes: 6 additions & 0 deletions packages/core/src/deploy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ export async function deploy<
strategyConfig,
maxFeePerGasLimit,
maxPriorityFeePerGas,
gasPrice,
disableFeeBumping,
}: {
config?: Partial<DeployConfig>;
artifactResolver: ArtifactResolver;
Expand All @@ -71,6 +73,8 @@ export async function deploy<
strategyConfig?: StrategyConfig[StrategyT];
maxFeePerGasLimit?: bigint;
maxPriorityFeePerGas?: bigint;
gasPrice?: bigint;
disableFeeBumping?: boolean;
}): Promise<DeploymentResult> {
const executionStrategy: ExecutionStrategy = resolveStrategy(
strategy,
Expand Down Expand Up @@ -117,6 +121,7 @@ export async function deploy<
const jsonRpcClient = new EIP1193JsonRpcClient(provider, {
maxFeePerGasLimit,
maxPriorityFeePerGas,
gasPrice,
});

const isAutominedNetwork = await checkAutominedNetwork(provider);
Expand All @@ -126,6 +131,7 @@ export async function deploy<
requiredConfirmations: isAutominedNetwork
? DEFAULT_AUTOMINE_REQUIRED_CONFIRMATIONS
: config.requiredConfirmations ?? defaultConfig.requiredConfirmations,
disableFeeBumping: disableFeeBumping ?? defaultConfig.disableFeeBumping,
...config,
};

Expand Down
1 change: 1 addition & 0 deletions packages/core/src/internal/defaultConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export const defaultConfig: DeployConfig = {
timeBeforeBumpingFees: 3 * 60 * 1_000,
maxFeeBumps: 4,
requiredConfirmations: 5,
disableFeeBumping: false,
};

/**
Expand Down
10 changes: 7 additions & 3 deletions packages/core/src/internal/deployer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,8 @@ export class Deployer {
ignitionModule.id,
this._deploymentDir,
isResumed,
this._config.maxFeeBumps
this._config.maxFeeBumps,
this._config.disableFeeBumping
);

const contracts =
Expand Down Expand Up @@ -188,7 +189,8 @@ export class Deployer {
this._config.requiredConfirmations,
this._config.timeBeforeBumpingFees,
this._config.maxFeeBumps,
this._config.blockPollingInterval
this._config.blockPollingInterval,
this._config.disableFeeBumping
);

deploymentState = await executionEngine.executeModule(
Expand Down Expand Up @@ -270,7 +272,8 @@ export class Deployer {
moduleId: string,
deploymentDir: string | undefined,
isResumed: boolean,
maxFeeBumps: number
maxFeeBumps: number,
disableFeeBumping: boolean
): void {
if (this._executionEventListener === undefined) {
return;
Expand All @@ -282,6 +285,7 @@ export class Deployer {
deploymentDir: deploymentDir ?? undefined,
isResumed,
maxFeeBumps,
disableFeeBumping,
});
}

Expand Down
6 changes: 4 additions & 2 deletions packages/core/src/internal/execution/execution-engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ export class ExecutionEngine {
private readonly _requiredConfirmations: number,
private readonly _millisecondBeforeBumpingFees: number,
private readonly _maxFeeBumps: number,
private readonly _blockPollingInterval: number
private readonly _blockPollingInterval: number,
private readonly _disableFeeBumping: boolean
) {}

/**
Expand Down Expand Up @@ -98,7 +99,8 @@ export class ExecutionEngine {
this._maxFeeBumps,
accounts,
deploymentParameters,
defaultSender
defaultSender,
this._disableFeeBumping
);

const futures = getFuturesFromModule(module);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ export class FutureProcessor {
private readonly _maxFeeBumps: number,
private readonly _accounts: string[],
private readonly _deploymentParameters: DeploymentParameters,
private readonly _defaultSender: string
private readonly _defaultSender: string,
private readonly _disableFeeBumping: boolean
) {}

/**
Expand Down Expand Up @@ -209,7 +210,9 @@ export class FutureProcessor {
this._transactionTrackingTimer,
this._requiredConfirmations,
this._millisecondBeforeBumpingFees,
this._maxFeeBumps
this._maxFeeBumps,
undefined,
this._disableFeeBumping
);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ export interface GetTransactionRetryConfig {
* of a transaction.
* @param maxFeeBumps The maximum number of times we can bump the fees of a transaction
* before considering the onchain interaction timed out.
* @param getTransactionRetryConfig This is really only a parameter to help with testing this function
* @param disableFeeBumping Disables fee bumping for all transactions.
* @returns A message indicating the result of checking the transactions of the latest
* network interaction.
*/
Expand All @@ -68,7 +70,8 @@ export async function monitorOnchainInteraction(
getTransactionRetryConfig: GetTransactionRetryConfig = {
maxRetries: 10,
retryInterval: 1000,
}
},
disableFeeBumping: boolean
): Promise<
| TransactionConfirmMessage
| OnchainInteractionBumpFeesMessage
Expand Down Expand Up @@ -143,7 +146,10 @@ export async function monitorOnchainInteraction(
return undefined;
}

if (lastNetworkInteraction.transactions.length > maxFeeBumps) {
if (
disableFeeBumping ||
lastNetworkInteraction.transactions.length > maxFeeBumps
) {
return {
type: JournalMessageType.ONCHAIN_INTERACTION_TIMEOUT,
futureId: exState.id,
Expand Down
37 changes: 32 additions & 5 deletions packages/core/src/internal/execution/jsonrpc-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,7 @@ export class EIP1193JsonRpcClient implements JsonRpcClient {
private readonly _config?: {
maxFeePerGasLimit?: bigint;
maxPriorityFeePerGas?: bigint;
gasPrice?: bigint;
}
) {}

Expand Down Expand Up @@ -643,9 +644,13 @@ export class EIP1193JsonRpcClient implements JsonRpcClient {
]);

// We prioritize EIP-1559 fees over legacy gasPrice fees, however,
// polygon (chainId 137) requires legacy gasPrice fees
// so we skip EIP-1559 logic in that case
if (latestBlock.baseFeePerGas !== undefined && chainId !== 137) {
// polygon (chainId 137) and polygon's amoy testnet (chainId 80002)
// both require legacy gasPrice fees so we skip EIP-1559 logic in those cases
if (
latestBlock.baseFeePerGas !== undefined &&
chainId !== 137 &&
chainId !== 80002
) {
// Support zero gas fee chains, such as a private instances
// of blockchains using Besu. We explicitly exclude BNB
// Smartchain (chainId 56) and its testnet (chainId 97)
Expand Down Expand Up @@ -674,6 +679,28 @@ export class EIP1193JsonRpcClient implements JsonRpcClient {
};
}

/**
* Polygon amoy testnet (chainId 80002) currently has a bug causing the
* `eth_gasPrice` RPC call to return an amount that is way too high.
* We hardcode the gas price for this chain for now until the bug is fixed.
* See: https://github.com/maticnetwork/bor/issues/1213
*
* Note that at the time of this implementation, the issue was autoclosed by a bot
* as a maintainer had not responded to the issue yet. Users continue to report
* the bug in the issue comments, however.
*
* All of that to say, when evaluating whether this logic is still needed in the future,
* it will likely be required to read through the issue above, rather than relying on the
* status of the github issue itself.
Comment on lines +688 to +694
Copy link
Member

Choose a reason for hiding this comment

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

This is great!

I mean, your comment helping our future selves, the bug is so sad 🤣

*/
if (chainId === 80002) {
return { gasPrice: 32000000000n };
}

if (this._config?.gasPrice !== undefined) {
return { gasPrice: this._config.gasPrice };
}

const response = await this._provider.request({
method: "eth_gasPrice",
params: [],
Expand All @@ -699,7 +726,7 @@ export class EIP1193JsonRpcClient implements JsonRpcClient {
}

try {
return await this._getMaxPrioirtyFeePerGas();
return await this._getMaxPriorityFeePerGas();
} catch {
// the max priority fee RPC call is not supported by
// this chain
Expand All @@ -708,7 +735,7 @@ export class EIP1193JsonRpcClient implements JsonRpcClient {
return DEFAULT_MAX_PRIORITY_FEE_PER_GAS;
}

private async _getMaxPrioirtyFeePerGas(): Promise<bigint> {
private async _getMaxPriorityFeePerGas(): Promise<bigint> {
const fee = await this._provider.request({
method: "eth_maxPriorityFeePerGas",
});
Expand Down
5 changes: 5 additions & 0 deletions packages/core/src/types/deploy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@ export interface DeployConfig {
* a transaction to be confirmed during Ignition execution.
*/
requiredConfirmations: number;

/**
* Disables fee bumping for all transactions.
*/
disableFeeBumping: boolean;
}

/**
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/types/execution-events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ export interface DeploymentStartEvent {
deploymentDir: string | undefined;
isResumed: boolean;
maxFeeBumps: number;
disableFeeBumping: boolean;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,8 @@ describe("Network interactions", () => {
requiredConfirmations,
millisecondBeforeBumpingFees,
maxFeeBumps,
testGetTransactionRetryConfig
testGetTransactionRetryConfig,
false
);

if (message === undefined) {
Expand Down Expand Up @@ -286,7 +287,8 @@ describe("Network interactions", () => {
requiredConfirmations,
millisecondBeforeBumpingFees,
maxFeeBumps,
testGetTransactionRetryConfig
testGetTransactionRetryConfig,
false
),
/IGN401: Error while executing test: all the transactions of its network interaction 1 were dropped\. Please try rerunning Hardhat Ignition\./
);
Expand Down
3 changes: 2 additions & 1 deletion packages/core/test/execution/future-processor/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,8 @@ export async function setupFutureProcessor(
100, // maxFeeBumps
exampleAccounts,
{},
getDefaultSender(exampleAccounts)
getDefaultSender(exampleAccounts),
false // disableFeeBumping
);

return { processor, storedDeployedAddresses };
Expand Down
5 changes: 5 additions & 0 deletions packages/hardhat-plugin/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ extendConfig((config, userConfig) => {
config.networks[networkName].ignition = {
maxFeePerGasLimit: userNetworkConfig.ignition?.maxFeePerGasLimit,
maxPriorityFeePerGas: userNetworkConfig.ignition?.maxPriorityFeePerGas,
gasPrice: userNetworkConfig.ignition?.gasPrice,
disableFeeBumping: userNetworkConfig.ignition?.disableFeeBumping,
};
});

Expand Down Expand Up @@ -320,6 +322,9 @@ ignitionScope
maxPriorityFeePerGas:
hre.config.networks[hre.network.name]?.ignition
.maxPriorityFeePerGas,
gasPrice: hre.config.networks[hre.network.name]?.ignition.gasPrice,
disableFeeBumping:
hre.config.networks[hre.network.name]?.ignition.disableFeeBumping,
});

try {
Expand Down
Loading
Loading