Skip to content
Merged
3 changes: 3 additions & 0 deletions packages/thirdweb/src/chains/types.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import type { FeeType } from "../transaction/prepare-transaction.js";

/**
* @chain
*/
Expand Down Expand Up @@ -26,6 +28,7 @@ export type ChainOptions = {
increaseZeroByteCount?: boolean;
};
faucets?: Array<string>;
feeType?: FeeType;
};

/**
Expand Down
50 changes: 32 additions & 18 deletions packages/thirdweb/src/gas/fee-data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@
import { eth_getBlockByNumber } from "../rpc/actions/eth_getBlockByNumber.js";
import { eth_maxPriorityFeePerGas } from "../rpc/actions/eth_maxPriorityFeePerGas.js";
import { getRpcClient } from "../rpc/rpc.js";
import type { PreparedTransaction } from "../transaction/prepare-transaction.js";
import type {
FeeType,
PreparedTransaction,
} from "../transaction/prepare-transaction.js";
import { resolvePromisedValue } from "../utils/promise/resolve-promised-value.js";
import { toUnits } from "../utils/units.js";
import { getGasPrice } from "./get-gas-price.js";
Expand Down Expand Up @@ -50,11 +53,13 @@
transaction: PreparedTransaction,
): Promise<FeeDataParams> {
// first check for explicit values
const [maxFeePerGas, maxPriorityFeePerGas, gasPrice] = await Promise.all([
resolvePromisedValue(transaction.maxFeePerGas),
resolvePromisedValue(transaction.maxPriorityFeePerGas),
resolvePromisedValue(transaction.gasPrice),
]);
const [maxFeePerGas, maxPriorityFeePerGas, gasPrice, feeType] =
await Promise.all([
resolvePromisedValue(transaction.maxFeePerGas),
resolvePromisedValue(transaction.maxPriorityFeePerGas),
resolvePromisedValue(transaction.gasPrice),
resolvePromisedValue(transaction.feeType),
Copy link
Member

Choose a reason for hiding this comment

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

so we have it on the chain, on the tx AND in the function itself? seems a bit much no?

Copy link
Contributor

@gregfromstl gregfromstl Feb 6, 2025

Choose a reason for hiding this comment

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

It should come from the chains DB if possible but the user override only on the transaction. User override takes precedence

Copy link
Contributor Author

Choose a reason for hiding this comment

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

for context, I felt this need on engine where I need to expose configuration that allows users to force legacy transactions on a chain. Especially important when engine sends nonce cancellation transactions which are not user triggered. (I could manage this in engine too, but felt like it could be a more general thing so the SDK would benefit from it)

Copy link
Member

Choose a reason for hiding this comment

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

yeah chain level makes sense to me

]);

// Exit early if the user explicitly provided enough options
if (maxFeePerGas !== undefined && maxPriorityFeePerGas !== undefined) {
Expand All @@ -63,6 +68,7 @@
maxPriorityFeePerGas,
};
}

if (typeof gasPrice === "bigint") {
return { gasPrice };
}
Expand All @@ -71,6 +77,7 @@
const defaultGasOverrides = await getDefaultGasOverrides(
transaction.client,
transaction.chain,
feeType,
);

if (transaction.chain.experimental?.increaseZeroByteCount) {
Expand Down Expand Up @@ -113,20 +120,27 @@
export async function getDefaultGasOverrides(
client: ThirdwebClient,
chain: Chain,
feeType?: FeeType,
) {
// if chain is in the force gas price list, always use gas price
if (!FORCE_GAS_PRICE_CHAIN_IDS.includes(chain.id)) {
const feeData = await getDynamicFeeData(client, chain);
if (
feeData.maxFeePerGas !== null &&
feeData.maxPriorityFeePerGas !== null
) {
return {
maxFeePerGas: feeData.maxFeePerGas,
maxPriorityFeePerGas: feeData.maxPriorityFeePerGas,
};
}
// give priority to the transaction fee type over the chain fee type
const resolvedFeeType = feeType ?? chain.feeType;
// if chain is configured to force legacy transactions or is in the legacy chain list
if (
resolvedFeeType === "legacy" ||
FORCE_GAS_PRICE_CHAIN_IDS.includes(chain.id)
) {
return {
gasPrice: await getGasPrice({ client, chain, percentMultiplier: 10 }),
};
}

Check warning on line 135 in packages/thirdweb/src/gas/fee-data.ts

View check run for this annotation

Codecov / codecov/patch

packages/thirdweb/src/gas/fee-data.ts#L132-L135

Added lines #L132 - L135 were not covered by tests
const feeData = await getDynamicFeeData(client, chain);
if (feeData.maxFeePerGas !== null && feeData.maxPriorityFeePerGas !== null) {
return {
maxFeePerGas: feeData.maxFeePerGas,
maxPriorityFeePerGas: feeData.maxPriorityFeePerGas,
};
}
// TODO: resolvedFeeType here could be "EIP1559", but we could not get EIP1559 fee data. should we throw?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@joaquim-verges wdyt about this btw

Copy link
Member

Choose a reason for hiding this comment

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

lets not throw, i think there's some cases where both type=1559 + gasprice are valid. polygon being one example IIRC

return {
gasPrice: await getGasPrice({ client, chain, percentMultiplier: 10 }),
};
Expand Down
3 changes: 3 additions & 0 deletions packages/thirdweb/src/transaction/prepare-transaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export type StaticPrepareTransactionOptions = {
maxFeePerGas?: bigint | undefined;
maxPriorityFeePerGas?: bigint | undefined;
maxFeePerBlobGas?: bigint | undefined;
feeType?: FeeType | undefined;
Copy link
Member

Choose a reason for hiding this comment

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

actually lets not add this, lets just do the chain one.

this prop is too similar the tx.type one which also indicates if its a legacy or 1559 tx

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we still would want a way to tell at the transaction level, hey please calculate the gas and send as legacy. If not this interface, we can use a different one, any thoughts?

Copy link
Member

Choose a reason for hiding this comment

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

we have type in SerializableTransaction already should map to that

nonce?: number | undefined;
extraGas?: bigint | undefined;
// eip7702
Expand All @@ -34,6 +35,8 @@ export type StaticPrepareTransactionOptions = {
};
};

export type FeeType = "legacy" | "eip1559";

export type EIP712TransactionOptions = {
// constant or user input
gasPerPubdata?: bigint | undefined;
Expand Down
Loading