Skip to content

feat: add explicit gas override to writeContract/deployContract #205

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

Open
ygd58 wants to merge 1 commit into
base: v2-dev
Choose a base branch
from
Open

feat: add explicit gas override to writeContract/deployContract #205

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
41 changes: 29 additions & 12 deletions src/contracts/actions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,7 @@ export const contractActions = (client: GenLayerClient<GenLayerChain>, publicCli
consensusMaxRotations?: number;
validUntil?: BigNumberish;
fees?: TransactionFeeOptions;
gas?: bigint;
}): Promise<`0x${string}`> => {
const {
account,
Expand All @@ -368,6 +369,7 @@ export const contractActions = (client: GenLayerClient<GenLayerChain>, publicCli
consensusMaxRotations = client.chain.defaultConsensusMaxRotations,
validUntil,
fees,
gas,
} = args;

const data = [calldata.encode(calldata.makeCalldataObject(functionName, callArgs, kwargs)), leaderOnly];
Expand All @@ -394,6 +396,7 @@ export const contractActions = (client: GenLayerClient<GenLayerChain>, publicCli
publicClient,
transactionVariants,
senderAccount,
gas,
});
},
/** Deploys a new intelligent contract to GenLayer. Returns the transaction hash. */
Expand All @@ -406,6 +409,7 @@ export const contractActions = (client: GenLayerClient<GenLayerChain>, publicCli
consensusMaxRotations?: number;
validUntil?: BigNumberish;
fees?: TransactionFeeOptions;
gas?: bigint;
}) => {
const {
account,
Expand All @@ -416,6 +420,7 @@ export const contractActions = (client: GenLayerClient<GenLayerChain>, publicCli
consensusMaxRotations = client.chain.defaultConsensusMaxRotations,
validUntil,
fees,
gas,
} = args;

const data = [
Expand Down Expand Up @@ -446,6 +451,7 @@ export const contractActions = (client: GenLayerClient<GenLayerChain>, publicCli
publicClient,
transactionVariants,
senderAccount,
gas,
});
},
/** Returns the active fee price policy used to build user-side caps. */
Expand Down Expand Up @@ -1988,11 +1994,18 @@ const _sendTransaction = async ({
publicClient,
transactionVariants,
senderAccount,
gas: gasOverride,
}: {
client: GenLayerClient<GenLayerChain>;
publicClient: PublicClient;
transactionVariants: EncodedTransactionVariant[];
senderAccount?: Account;
/**
* Explicit outer EVM gas limit, bypassing `eth_estimateGas` and the
* automatic headroom below. Used as-is with no further markup, since an
* explicit override is the caller stating what to use.
*/
gas?: bigint;
}) => {
if (!client.chain.consensusMainContract?.address) {
throw new Error(`Consensus main contract address not found in chain config for "${client.chain.name}".`);
Expand Down Expand Up @@ -2046,18 +2059,22 @@ const _sendTransaction = async ({
const sendWithEncodedData = async (transactionVariant: EncodedTransactionVariant) => {
let estimatedGas: bigint;
let gasEstimationError: string | undefined;
try {
estimatedGas = await client.estimateTransactionGas({
from: validatedSenderAccount.address,
to: client.chain.consensusMainContract?.address as Address,
data: transactionVariant.encodedData,
value: transactionVariant.value,
});
estimatedGas = withTransactionGasHeadroom(estimatedGas);
} catch (err) {
gasEstimationError = stringifyRpcError(err);
console.error("Gas estimation failed, using default 200_000:", err);
estimatedGas = 200_000n;
if (gasOverride !== undefined) {
estimatedGas = gasOverride;
} else {
Comment on lines +2062 to +2064

@coderabbitai coderabbitai Bot Aug 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Reject non-positive gas overrides.

A caller can pass 0n or a negative bigint. A negative value reaches the external-wallet request as an invalid value such as 0x-1. A zero value cannot execute an EVM transaction.

Validate gasOverride > 0n in _sendTransaction before transaction preparation. Add tests for 0n and a negative value.

Proposed fix 
+  if (gasOverride !== undefined && gasOverride <= 0n) {
+    throw new Error("gas must be greater than zero.");
+  }
+
   const sendWithEncodedData = async (transactionVariant: EncodedTransactionVariant) => {
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/contracts/actions.ts` around lines 2062 - 2064, Update _sendTransaction
to validate gasOverride before transaction preparation, accepting only bigint
values greater than 0n and rejecting 0n or negative overrides with the existing
validation error behavior. Add coverage for both zero and negative gasOverride
values.

try {
estimatedGas = await client.estimateTransactionGas({
from: validatedSenderAccount.address,
to: client.chain.consensusMainContract?.address as Address,
data: transactionVariant.encodedData,
value: transactionVariant.value,
});
estimatedGas = withTransactionGasHeadroom(estimatedGas);
} catch (err) {
gasEstimationError = stringifyRpcError(err);
console.error("Gas estimation failed, using default 200_000:", err);
estimatedGas = 200_000n;
}
}

// For local accounts, build transaction request directly to avoid viem's
Expand Down
10 changes: 10 additions & 0 deletions src/types/clients.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,14 @@ export type GenLayerClient<TGenLayerChain extends GenLayerChain> = Omit<
consensusMaxRotations?: number;
validUntil?: BigNumberish;
fees?: TransactionFeeOptions;
/**
* Explicit outer EVM gas limit for the `addTransaction` call, bypassing
* `eth_estimateGas` and the automatic safety headroom entirely. Useful
* when gas estimation is unreliable for a given RPC/network and the
* estimated-but-exact limit reverts the outer transaction before GenVM
* even sees it.
*/
gas?: bigint;
}) => Promise<any>;
simulateWriteContract: <
RawReturn extends boolean | undefined = undefined,
Expand Down Expand Up @@ -110,6 +118,8 @@ export type GenLayerClient<TGenLayerChain extends GenLayerChain> = Omit<
consensusMaxRotations?: number;
validUntil?: BigNumberish;
fees?: TransactionFeeOptions;
/** Explicit outer EVM gas limit, bypassing `eth_estimateGas`. See `writeContract`'s `gas` option. */
gas?: bigint;
}) => Promise<`0x${string}`>;
getTransaction: (args: {hash: TransactionHash}) => Promise<GenLayerTransaction>;
getCurrentNonce: (args: {address: Address}) => Promise<number>;
Expand Down
70 changes: 70 additions & 0 deletions tests/contracts-actions.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -461,6 +461,76 @@ describe("contractActions addTransaction ABI compatibility", () => {
expect(encodedData.slice(0, 10)).toBe(selectorForV5);
});

it("uses the explicit gas override as-is, bypassing eth_estimateGas entirely (#402)", async () => {
const signTransaction = vi.fn().mockRejectedValue(new Error("stop_after_encoding"));
const {actions, estimateTransactionGas} = setupWriteContractHarness({
initialAbi: ADD_TRANSACTION_ABI_V5,
signTransactionMock: signTransaction,
});

await expect(
actions.writeContract({
address: RECIPIENT_ADDRESS,
functionName: "ping",
value: 0n,
gas: 2_000_000n,
}),
).rejects.toThrow("stop_after_encoding");

// eth_estimateGas must not be called at all when an override is supplied —
// an exact estimate is exactly the failure mode #402 reports, so the
// override path must not go anywhere near it.
expect(estimateTransactionGas).not.toHaveBeenCalled();

const txRequest = signTransaction.mock.calls[0][0];
expect(txRequest.gas).toBe(2_000_000n);
});

it("still estimates and applies headroom when no gas override is given (#402)", async () => {
const signTransaction = vi.fn().mockRejectedValue(new Error("stop_after_encoding"));
const {actions, estimateTransactionGas} = setupWriteContractHarness({
initialAbi: ADD_TRANSACTION_ABI_V5,
signTransactionMock: signTransaction,
});
estimateTransactionGas.mockResolvedValue(1_319_997n);

await expect(
actions.writeContract({
address: RECIPIENT_ADDRESS,
functionName: "ping",
value: 0n,
}),
).rejects.toThrow("stop_after_encoding");

expect(estimateTransactionGas).toHaveBeenCalledTimes(1);

const txRequest = signTransaction.mock.calls[0][0];
// 1_319_997 * 20_000bps / 10_000 = 2_639_994 — well above the 2_000_000
// that #402 confirmed succeeds against the exact 1_319_997 estimate that
// reverted twice.
expect(txRequest.gas).toBe(2_639_994n);
});

it("threads the gas override through deployContract the same way as writeContract (#402)", async () => {
const signTransaction = vi.fn().mockRejectedValue(new Error("stop_after_encoding"));
const {actions, estimateTransactionGas} = setupWriteContractHarness({
initialAbi: ADD_TRANSACTION_ABI_V5,
signTransactionMock: signTransaction,
});

await expect(
actions.deployContract({
code: "0x1234",
args: [],
gas: 3_000_000n,
}),
).rejects.toThrow("stop_after_encoding");

expect(estimateTransactionGas).not.toHaveBeenCalled();
const txRequest = signTransaction.mock.calls[0][0];
expect(txRequest.gas).toBe(3_000_000n);
});

it("encodes addTransaction with 6 args when ABI has 6 inputs", async () => {
const {actions, estimateTransactionGas} = setupWriteContractHarness({
initialAbi: ADD_TRANSACTION_ABI_V6,
Expand Down
Loading