Skip to content

Fixes for gas estimation when performing SC transactions #672

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

Merged
popenta merged 1 commit into from
Oct 20, 2025
Merged

Fixes for gas estimation when performing SC transactions #672

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
47 changes: 46 additions & 1 deletion src/multisig/multisigController.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import { assert } from "chai";
import { Account } from "../accounts";
import { Address, CodeMetadata, SmartContractQueryResponse } from "../core";
import { loadAbiRegistry, MockNetworkProvider } from "../testutils";
import { GasLimitEstimator } from "../gasEstimator";
import { ProxyNetworkProvider } from "../networkProviders";
import { getTestWalletsPath, loadAbiRegistry, loadContractCode, MockNetworkProvider } from "../testutils";
import { MultisigController } from "./multisigController";
import * as resources from "./resources";

Expand All @@ -20,6 +23,48 @@ describe("test multisig controller query methods", () => {
});
});

it("should create transaction for deploy multisig contract", async function () {
const alice = await Account.newFromPem(`${getTestWalletsPath()}/alice.pem`);

const boardMemberOne = Address.newFromBech32("erd1spyavw0956vq68xj8y4tenjpq2wd5a9p2c6j8gsz7ztyrnpxrruqzu66jx");
const boardMemberTwo = Address.newFromBech32("erd1k2s324ww2g0yj38qn2ch2jwctdy8mnfxep94q9arncc6xecg3xaq6mjse8");

const board = [boardMemberOne, boardMemberTwo];

const bytecode = await loadContractCode("src/testdata/multisig-full.wasm");
const abi = await loadAbiRegistry("src/testdata/multisig-full.abi.json");

const networkProvider = new ProxyNetworkProvider("https://devnet-gateway.multiversx.com");
const gasLimitEstimator = new GasLimitEstimator({ networkProvider: networkProvider });

const controller = new MultisigController({
chainID: "D",
networkProvider: networkProvider,
abi: abi,
gasLimitEstimator: gasLimitEstimator,
});
Comment on lines +37 to +45

Copilot AI Oct 20, 2025

Copy link

Choose a reason for hiding this comment

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

The test uses a live network provider, making the spec non-deterministic and potentially flaky (also fetching account nonce from network). Use a mock provider that simulates gas estimation and account queries, or convert this into an integration test that only runs when an env flag is set.

Copilot uses AI. Check for mistakes.

alice.nonce = (await networkProvider.getAccount(alice.address)).nonce;

const transaction = await controller.createTransactionForDeploy(alice, alice.getNonceThenIncrement(), {
bytecode: bytecode,
quorum: 2,
board,
});
const bytecodeHex = Buffer.from(bytecode).toString("hex");
assert.equal(transaction.sender.toBech32(), alice.address.toBech32());
assert.equal(transaction.receiver.toBech32(), "erd1qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq6gq4hu");
assert.equal(transaction.value, 0n);
assert.equal(transaction.chainID, "D");
assert.isTrue(transaction.gasLimit > 0n);
assert.deepEqual(
Buffer.from(transaction.data),
Buffer.from(
`${bytecodeHex}@0500@0504@02@8049d639e5a6980d1cd2392abcce41029cda74a1563523a202f09641cc2618f8@b2a11555ce521e4944e09ab17549d85b487dcd26c84b5017a39e31a3670889ba`,
),
);
});

it("getQuorum returns the quorum value", async function () {
networkProvider.mockQueryContractOnFunction(
"getQuorum",
Expand Down
20 changes: 20 additions & 0 deletions src/multisig/multisigController.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ export class MultisigController extends BaseController {
nonce: bigint,
options: resources.DeployMultisigContractInput & BaseControllerInput,
): Promise<Transaction> {
options.gasLimit = options.gasLimit ? options.gasLimit : 0n;

Copilot AI Oct 20, 2025

Copy link

Choose a reason for hiding this comment

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

This gasLimit defaulting logic is duplicated across many methods in this controller. Consider centralizing it (e.g., a small helper like ensureGasLimit(options) or handling it once in setupAndSignTransaction), and use the nullish coalescing assignment form options.gasLimit ??= 0n; for clearer intent.

Copilot uses AI. Check for mistakes.
const transaction = await this.multisigFactory.createTransactionForDeploy(sender.address, options);

transaction.guardian = options.guardian ?? Address.empty();
Expand Down Expand Up @@ -318,6 +319,7 @@ export class MultisigController extends BaseController {
nonce: bigint,
options: resources.ProposeAddBoardMemberInput & BaseControllerInput,
): Promise<Transaction> {
options.gasLimit = options.gasLimit ? options.gasLimit : 0n;
const transaction = await this.multisigFactory.createTransactionForProposeAddBoardMember(
sender.address,
options,
Expand All @@ -336,6 +338,7 @@ export class MultisigController extends BaseController {
nonce: bigint,
options: resources.ProposeAddProposerInput & BaseControllerInput,
): Promise<Transaction> {
options.gasLimit = options.gasLimit ? options.gasLimit : 0n;
const transaction = await this.multisigFactory.createTransactionForProposeAddProposer(sender.address, options);

await this.setupAndSignTransaction(transaction, options, nonce, sender);
Expand All @@ -351,6 +354,7 @@ export class MultisigController extends BaseController {
nonce: bigint,
options: resources.ProposeRemoveUserInput & BaseControllerInput,
): Promise<Transaction> {
options.gasLimit = options.gasLimit ? options.gasLimit : 0n;
const transaction = await this.multisigFactory.createTransactionForProposeRemoveUser(sender.address, options);

await this.setupAndSignTransaction(transaction, options, nonce, sender);
Expand All @@ -366,6 +370,7 @@ export class MultisigController extends BaseController {
nonce: bigint,
options: resources.ProposeChangeQuorumInput & BaseControllerInput,
): Promise<Transaction> {
options.gasLimit = options.gasLimit ? options.gasLimit : 0n;
const transaction = await this.multisigFactory.createTransactionForProposeChangeQuorum(sender.address, options);

await this.setupAndSignTransaction(transaction, options, nonce, sender);
Expand All @@ -390,6 +395,7 @@ export class MultisigController extends BaseController {
nonce: bigint,
options: resources.ActionInput & BaseControllerInput,
): Promise<Transaction> {
options.gasLimit = options.gasLimit ? options.gasLimit : 0n;
const transaction = await this.multisigFactory.createTransactionForSignAction(sender.address, options);

await this.setupAndSignTransaction(transaction, options, nonce, sender);
Expand All @@ -405,6 +411,7 @@ export class MultisigController extends BaseController {
nonce: bigint,
options: resources.ActionInput & BaseControllerInput,
): Promise<Transaction> {
options.gasLimit = options.gasLimit ? options.gasLimit : 0n;
const transaction = await this.multisigFactory.createTransactionForPerformAction(sender.address, options);

await this.setupAndSignTransaction(transaction, options, nonce, sender);
Expand All @@ -429,6 +436,7 @@ export class MultisigController extends BaseController {
nonce: bigint,
options: resources.ActionInput & BaseControllerInput,
): Promise<Transaction> {
options.gasLimit = options.gasLimit ? options.gasLimit : 0n;
const transaction = await this.multisigFactory.createTransactionForUnsign(sender.address, options);

await this.setupAndSignTransaction(transaction, options, nonce, sender);
Expand All @@ -444,6 +452,7 @@ export class MultisigController extends BaseController {
nonce: bigint,
options: resources.ActionInput & BaseControllerInput,
): Promise<Transaction> {
options.gasLimit = options.gasLimit ? options.gasLimit : 0n;
const transaction = await this.multisigFactory.createTransactionForDiscardAction(sender.address, options);

await this.setupAndSignTransaction(transaction, options, nonce, sender);
Expand All @@ -459,6 +468,7 @@ export class MultisigController extends BaseController {
nonce: bigint,
options: resources.DepositExecuteInput & BaseControllerInput,
): Promise<Transaction> {
options.gasLimit = options.gasLimit ? options.gasLimit : 0n;
const transaction = await this.multisigFactory.createTransactionForDeposit(sender.address, options);

await this.setupAndSignTransaction(transaction, options, nonce, sender);
Expand All @@ -474,6 +484,7 @@ export class MultisigController extends BaseController {
nonce: bigint,
options: resources.ProposeTransferExecuteInput & BaseControllerInput,
): Promise<Transaction> {
options.gasLimit = options.gasLimit ? options.gasLimit : 0n;
const transaction = await this.multisigFactory.createTransactionForProposeTransferExecute(
sender.address,
options,
Expand All @@ -492,6 +503,7 @@ export class MultisigController extends BaseController {
nonce: bigint,
options: resources.ProposeTransferExecuteEsdtInput & BaseControllerInput,
): Promise<Transaction> {
options.gasLimit = options.gasLimit ? options.gasLimit : 0n;
const transaction = await this.multisigFactory.createTransactionForProposeTransferExecuteEsdt(
sender.address,
options,
Expand All @@ -510,6 +522,7 @@ export class MultisigController extends BaseController {
nonce: bigint,
options: resources.ProposeAsyncCallInput & BaseControllerInput,
): Promise<Transaction> {
options.gasLimit = options.gasLimit ? options.gasLimit : 0n;
const transaction = await this.multisigFactory.createTransactionForProposeAsyncCall(sender.address, options);

await this.setupAndSignTransaction(transaction, options, nonce, sender);
Expand All @@ -525,6 +538,7 @@ export class MultisigController extends BaseController {
nonce: bigint,
options: resources.ProposeContractDeployFromSourceInput & BaseControllerInput,
): Promise<Transaction> {
options.gasLimit = options.gasLimit ? options.gasLimit : 0n;
const transaction = await this.multisigFactory.createTransactionForProposeContractDeployFromSource(
sender.address,
options,
Expand All @@ -543,6 +557,7 @@ export class MultisigController extends BaseController {
nonce: bigint,
options: resources.ProposeContractUpgradeFromSourceInput & BaseControllerInput,
): Promise<Transaction> {
options.gasLimit = options.gasLimit ? options.gasLimit : 0n;
const transaction = await this.multisigFactory.createTransactionForProposeContractUpgradeFromSource(
sender.address,
options,
Expand All @@ -561,6 +576,7 @@ export class MultisigController extends BaseController {
nonce: bigint,
options: resources.GroupInput & BaseControllerInput,
): Promise<Transaction> {
options.gasLimit = options.gasLimit ? options.gasLimit : 0n;
const transaction = await this.multisigFactory.createTransactionForSignBatch(sender.address, options);

await this.setupAndSignTransaction(transaction, options, nonce, sender);
Expand All @@ -576,6 +592,7 @@ export class MultisigController extends BaseController {
nonce: bigint,
options: resources.ActionInput & BaseControllerInput,
): Promise<Transaction> {
options.gasLimit = options.gasLimit ? options.gasLimit : 0n;
const transaction = await this.multisigFactory.createTransactionForSignAndPerform(sender.address, options);

await this.setupAndSignTransaction(transaction, options, nonce, sender);
Expand All @@ -591,6 +608,7 @@ export class MultisigController extends BaseController {
nonce: bigint,
options: resources.UnsignForOutdatedBoardMembersInput & BaseControllerInput,
): Promise<Transaction> {
options.gasLimit = options.gasLimit ? options.gasLimit : 0n;
const transaction = await this.multisigFactory.createTransactionForUnsignForOutdatedBoardMembers(
sender.address,
options,
Expand All @@ -609,6 +627,7 @@ export class MultisigController extends BaseController {
nonce: bigint,
options: resources.GroupInput & BaseControllerInput,
): Promise<Transaction> {
options.gasLimit = options.gasLimit ? options.gasLimit : 0n;
const transaction = await this.multisigFactory.createTransactionForPerformBatch(sender.address, options);

await this.setupAndSignTransaction(transaction, options, nonce, sender);
Expand All @@ -624,6 +643,7 @@ export class MultisigController extends BaseController {
nonce: bigint,
options: resources.DiscardBatchInput & BaseControllerInput,
): Promise<Transaction> {
options.gasLimit = options.gasLimit ? options.gasLimit : 0n;
const transaction = await this.multisigFactory.createTransactionForDiscardBatch(sender.address, options);

await this.setupAndSignTransaction(transaction, options, nonce, sender);
Expand Down
24 changes: 23 additions & 1 deletion src/smartContracts/smartContractController.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ import { Abi, BigUIntValue, BooleanValue, BytesValue, Tuple, U16Value, U64Value
import { Account } from "../accounts";
import { Address, SmartContractQueryResponse } from "../core";
import { GasLimitEstimator } from "../gasEstimator";
import { MockNetworkProvider, getTestWalletsPath, loadAbiRegistry } from "../testutils";
import { ProxyNetworkProvider } from "../networkProviders";
import { MockNetworkProvider, getTestWalletsPath, loadAbiRegistry, loadContractCode } from "../testutils";
import { bigIntToBuffer } from "../tokenOperations/codec";
import { SmartContractController } from "./smartContractController";

Expand Down Expand Up @@ -276,5 +277,26 @@ describe("test smart contract queries controller", () => {

assert.equal(transaction.gasLimit, 123456789n);
});

it("should estimate gas using gasLimitEstimator", async function () {
const alice = await Account.newFromPem(`${getTestWalletsPath()}/alice.pem`);
const networkProvider = new ProxyNetworkProvider("https://devnet-gateway.multiversx.com");

const gasLimitEstimator = new GasLimitEstimator({ networkProvider: networkProvider });
const controller = new SmartContractController({
chainID: "D",
networkProvider: networkProvider,
gasLimitEstimator: gasLimitEstimator,
Comment on lines +283 to +289

Copilot AI Oct 20, 2025

Copy link

Choose a reason for hiding this comment

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

This test depends on a live devnet endpoint, which can introduce flakiness and external dependencies in unit tests. Prefer a mock/fake provider (extend MockNetworkProvider to support the estimation path) or mark/move this as an integration test gated behind an environment flag.

Copilot uses AI. Check for mistakes.
});

const bytecode = await loadContractCode("src/testdata/adder.wasm");

const transaction = await controller.createTransactionForDeploy(alice, 0n, {
bytecode: bytecode,
arguments: [new BigUIntValue(0)],
});

assert.isTrue(transaction.gasLimit > 0n);
});
});
});
9 changes: 6 additions & 3 deletions src/smartContracts/smartContractController.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,9 @@ export class SmartContractController extends BaseController {
nonce: bigint,
options: resources.ContractDeployInput & BaseControllerInput,
): Promise<Transaction> {
const transaction = await this.factory.createTransactionForDeploy(sender.address, options);
options.gasLimit = options.gasLimit ? options.gasLimit : 0n;

Copilot AI Oct 20, 2025

Copy link

Choose a reason for hiding this comment

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

Use the nullish coalescing assignment operator for clarity and correct semantics on optional values. Replace with options.gasLimit ??= 0n; and apply the same change to similar occurrences in this file.

Suggested change
options.gasLimit = options.gasLimit ? options.gasLimit : 0n;
options.gasLimit ??= 0n;

Copilot uses AI. Check for mistakes.

const transaction = await this.factory.createTransactionForDeploy(sender.address, options);
await this.setupAndSignTransaction(transaction, options, nonce, sender);

return transaction;
Expand All @@ -69,8 +70,9 @@ export class SmartContractController extends BaseController {
nonce: bigint,
options: resources.ContractUpgradeInput & BaseControllerInput,
): Promise<Transaction> {
const transaction = await this.factory.createTransactionForUpgrade(sender.address, options);
options.gasLimit = options.gasLimit ? options.gasLimit : 0n;

const transaction = await this.factory.createTransactionForUpgrade(sender.address, options);
await this.setupAndSignTransaction(transaction, options, nonce, sender);

return transaction;
Expand All @@ -81,8 +83,9 @@ export class SmartContractController extends BaseController {
nonce: bigint,
options: resources.ContractExecuteInput & BaseControllerInput,
): Promise<Transaction> {
const transaction = await this.factory.createTransactionForExecute(sender.address, options);
options.gasLimit = options.gasLimit ? options.gasLimit : 0n;

const transaction = await this.factory.createTransactionForExecute(sender.address, options);
await this.setupAndSignTransaction(transaction, options, nonce, sender);

return transaction;
Expand Down
4 changes: 2 additions & 2 deletions src/smartContracts/smartContractTransactionsFactory.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ describe("test smart contract transactions factory", function () {
assert.fail("Expected error was not thrown");
} catch (err) {
assert.instanceOf(err, Err);
assert.match(err.message, /Can't convert args to TypedValues/);
assert.match((err as Error).message, /Can't convert args to TypedValues/);
}
});

Expand Down Expand Up @@ -570,7 +570,7 @@ describe("test smart contract transactions factory", function () {
assert.fail("Expected error was not thrown");
} catch (err) {
assert.instanceOf(err, Err);
assert.match(err.message, /Can't convert args to TypedValues/);
assert.match((err as Error).message, /Can't convert args to TypedValues/);
}
});

Expand Down
4 changes: 2 additions & 2 deletions src/transfers/transferTransactionsFactory.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ describe("test transfer transactions factory", function () {
assert.fail("Expected error was not thrown");
} catch (err) {
assert.instanceOf(err, ErrBadUsage);
assert.match(err.message, /No token transfer has been provided/);
assert.match((err as Error).message, /No token transfer has been provided/);
}
});

Expand Down Expand Up @@ -181,7 +181,7 @@ describe("test transfer transactions factory", function () {

assert.fail("Expected error was not thrown");
} catch (err) {
assert.match(err.message, /Can't set data field when sending esdt tokens/);
assert.match((err as Error).message, /Can't set data field when sending esdt tokens/);
}
});

Expand Down
Loading