Skip to content

Commit

Permalink
problem: always uses the default Gas Limit value for a new tx
Browse files Browse the repository at this point in the history
solution: run eth_estimateGas during the tx setup
  • Loading branch information
splix committed Dec 18, 2024
1 parent 7ecc9f6 commit 9bef05b
Show file tree
Hide file tree
Showing 10 changed files with 119 additions and 79 deletions.
2 changes: 1 addition & 1 deletion packages/core/src/BackendApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ export interface BackendApi {
describeAddress(blockchain: BlockchainCode, address: string): Promise<AddressApi.DescribeResponse>;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
estimateFee(blockchain: BlockchainCode, blocks: number, mode: EstimationMode): Promise<any>;
estimateTxCost(blockchain: BlockchainCode, tx: EthereumBasicTransaction): Promise<number>;
estimateGasLimit(blockchain: BlockchainCode, tx: EthereumBasicTransaction): Promise<number>;
getBalance(address: string, asset: AnyAsset, includeUtxo?: boolean): Promise<AddressBalance[]>;
getBtcTx(blockchain: BlockchainCode, hash: string): Promise<BitcoinRawTransaction | null>;
getEthReceipt(blockchain: BlockchainCode, hash: string): Promise<EthereumRawReceipt | null>;
Expand Down
3 changes: 3 additions & 0 deletions packages/core/src/IpcCommands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@ export enum IpcCommands {
// Backend API
DESCRIBE_ADDRESS = 'describe-address',
ESTIMATE_FEE = 'estimate-fee',

// estimate the gas limit for the transaction
ESTIMATE_TX = 'estimate-tx',

GET_BALANCE = 'get-balance',
GET_BTC_TX = 'get-btc-tx',
GET_ETH_RECEIPT = 'get-eth-receipt',
Expand Down
15 changes: 6 additions & 9 deletions packages/react-app/src/common/EthTxSettings/EthTxSettings.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ const EthTxSettings: React.FC<OwnProps> = ({

const [currentUseStdMaxGasPrice, setCurrentUseStdMaxGasPrice] = React.useState(true);
const [currentUseStdPriorityGasPrice, setCurrentUseStdPriorityGasPrice] = React.useState(true);
const [gasLimit, setGasLimit] = React.useState(estimatedGasLimit);
const [overriddenGasLimit, setOverriddenGasLimit] = React.useState<number | undefined>(undefined);

const toWei = (decimal: number): WeiAny => WeiAny.createFor(decimal, stdMaxGasPrice.units, factory, gasPriceUnit);

Expand Down Expand Up @@ -208,13 +208,8 @@ const EthTxSettings: React.FC<OwnProps> = ({
};

const handleGasLimitOverride = (value?: number) => {
if (value != undefined) {
setGasLimit(value);
onGasLimitChange(value);
} else {
setGasLimit(estimatedGasLimit);
onGasLimitChange(estimatedGasLimit);
}
setOverriddenGasLimit(estimatedGasLimit);
onGasLimitChange(estimatedGasLimit);
}

const maxGasPriceByUnit = maxGasPrice.getNumberByUnit(gasPriceUnit).toFixed(2);
Expand All @@ -225,6 +220,8 @@ const EthTxSettings: React.FC<OwnProps> = ({
const showMaxRange = lowMaxGasPrice.isPositive() && highMaxGasPrice.isPositive();
const showPriorityRange = lowPriorityGasPrice.isPositive() && highPriorityGasPrice.isPositive();

const currentGasLimit = overriddenGasLimit ?? estimatedGasLimit;

return (
<FormAccordion
title={
Expand All @@ -233,7 +230,7 @@ const EthTxSettings: React.FC<OwnProps> = ({
{showEip1559 ? 'EIP-1559' : 'Basic Type'} / {maxGasPriceByUnit} {gasPriceUnit.toString()}
{showEip1559 ? ' Max Gas Price' : ' Gas Price'}
{showEip1559 ? ` / ${priorityGasPriceByUnit} ${gasPriceUnit.toString()} Priority Gas Price` : null}
/ {gasLimit} gas
/ {currentGasLimit} gas
</FormRow>
}
>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -294,9 +294,11 @@ export default connect<StateProps, DispatchProps, OwnProps, IState>(
(dispatch: any, { initialAllowance, storedTx }) => ({
prepareTransaction({ action, entries, entry }) {
dispatch(txStash.actions.prepareTransaction({ action, entries, entry, initialAllowance, storedTx }));
dispatch(txStash.actions.estimateGasLimit());
},
setAsset(asset) {
dispatch(txStash.actions.setAsset(asset));
dispatch(txStash.actions.estimateGasLimit());
},
setEntry(entry, ownerAddress) {
dispatch(txStash.actions.setEntry(entry, ownerAddress));
Expand All @@ -306,6 +308,7 @@ export default connect<StateProps, DispatchProps, OwnProps, IState>(
},
setTransaction(tx) {
dispatch(txStash.actions.setTransaction(tx));
dispatch(txStash.actions.estimateGasLimit());
},
}),
)(SetupTransaction);
2 changes: 1 addition & 1 deletion packages/react-app/stories/__mocks__/backendMock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ export class BackendMock implements BackendApi {
return Promise.resolve(0);
}

estimateTxCost(): Promise<number> {
estimateGasLimit(): Promise<number> {
return Promise.resolve(0);
}

Expand Down
2 changes: 1 addition & 1 deletion packages/store/src/BackendApiInvoker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export class BackendApiInvoker implements BackendApi {
return ipcRenderer.invoke(IpcCommands.ESTIMATE_FEE, blockchain, blocks, mode);
}

estimateTxCost(blockchain: BlockchainCode, tx: EthereumBasicTransaction): Promise<number> {
estimateGasLimit(blockchain: BlockchainCode, tx: EthereumBasicTransaction): Promise<number> {
return ipcRenderer.invoke(IpcCommands.ESTIMATE_TX, blockchain, tx);
}

Expand Down
61 changes: 0 additions & 61 deletions packages/store/src/transaction/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -200,34 +200,6 @@ export function broadcastTx({
};
}

// eslint-disable-next-line @typescript-eslint/no-explicit-any
export function estimateFee(blockchain: BlockchainCode, blocks: number, mode: EstimationMode): Dispatched<any> {
return (dispatch, getState, extra) => {
return extra.backendApi.estimateFee(blockchain, blocks, mode);
};
}

export function estimateGas(blockchain: BlockchainCode, tx: EthereumTransaction): Dispatched<number> {
return (dispatch, getState, extra) => {
const { data, from, gasPrice, maxGasPrice, priorityGasPrice, to, type, value } = tx;

if (to == null) {
return -1;
}

const basicTx: EthereumBasicTransaction = { data, from, to, value: toHex(value.number) };

if (type === EthereumTransactionType.EIP1559) {
basicTx.maxFeePerGas = toHex(maxGasPrice?.number);
basicTx.maxPriorityFeePerGas = toHex(priorityGasPrice?.number);
} else {
basicTx.gasPrice = toHex(gasPrice?.number);
}

return extra.backendApi.estimateTxCost(blockchain, basicTx);
};
}

function sortBigNumber(first: BigNumber, second: BigNumber): number {
if (first.eq(second)) {
return 0;
Expand Down Expand Up @@ -581,39 +553,6 @@ function verifySender(expected: string): (txid: string, raw: string, chain: Bloc
});
}

/**
* @deprecated Should be replaced by unified logic in new UI
*/
export function signEthereumTransaction(
entryId: string,
transaction: EthereumTransaction,
password: string | undefined,
): Dispatched<SignData | undefined> {
return (dispatch, getState, extra) => {
const callSignTx = (tx: EthereumTransaction): Promise<SignData> =>
signEthTx(entryId, tx, extra.api.vault, password)
.then(({ raw, txid }) => verifySender(tx.from)(txid, raw, tx.blockchain))
.then(({ raw: signed, txid: txId }) => ({ entryId, signed, tx, txId, blockchain: tx.blockchain }));

if (transaction.nonce == null) {
return extra.backendApi
.getNonce(transaction.blockchain, transaction.from)
.then((nonce) => callSignTx({ ...transaction, nonce }))
.catch((error) => {
dispatch(showError(error));

return undefined;
});
}

return callSignTx(transaction).catch((error) => {
dispatch(showError(error));

return undefined;
});
};
}

export function signTx(unsiged: UnsignedTx, entryId: EntryId, password?: string): Dispatched<SignedTx> {
return (dispatch, getState, extra) => extra.api.vault.signTx(entryId, unsiged, password);
}
Expand Down
80 changes: 75 additions & 5 deletions packages/store/src/txstash/actions.ts
Original file line number Diff line number Diff line change
@@ -1,24 +1,33 @@
import { SignedTx, WalletEntry } from '@emeraldpay/emerald-vault-core';
import { BlockchainCode, workflow } from '@emeraldwallet/core';
import { Dispatched } from '../types';
import { getHandler } from './handler';
import {SignedTx, WalletEntry} from '@emeraldpay/emerald-vault-core';
import {
BlockchainCode,
EthereumBasicTransaction,
EthereumTransaction,
TokenRegistry,
workflow
} from '@emeraldwallet/core';
import {Dispatched} from '../types';
import {getHandler} from './handler';
import {
ActionTypes,
CreateTxStage,
FeeRange,
FeeRange, moduleName,
ResetAction,
SetAssetAction,
SetChangeAddressAction,
SetEntryAction,
SetFeeLoadingAction,
SetFeeRangeAction,
SetGasLimitAction,
SetPreparingAction,
SetSignedAction,
SetStageAction,
SetTransactionAction,
SetTransactionFeeAction,
TxOrigin,
} from './types';
import {getTokens} from "../application/selectors";
import {Wei} from "@emeraldpay/bigamount-crypto";

export function prepareTransaction({ action, entries, entry, initialAllowance, storedTx }: TxOrigin): Dispatched<void> {
return (dispatch, getState, extra) => {
Expand Down Expand Up @@ -101,3 +110,64 @@ export function setTransactionFee(transactionFee: FeeRange): SetTransactionFeeAc
payload: { transactionFee },
};
}

export function estimateGasLimit(): Dispatched<void | SetGasLimitAction> {
return (dispatch, getState, extra) => {
let creatingTx = getState()[moduleName].transaction;

if (!creatingTx) {
return;
}

// In general, we want to estimate it only for transactions where it can change depending on tx details.
// Which is basically when it makes a tx to a contract or with a non-empty data. Note that for a standard Ether Transfer it may target a contract address as well.
// So basically it's everything expect a speedup (nothing changes except the price) and cancel (always 21_000 gas as we transfer to ourselves)
const gasIsDefined = creatingTx.meta.type in [
workflow.TxMetaType.ERC20_CANCEL,
workflow.TxMetaType.ERC20_SPEEDUP,
workflow.TxMetaType.ETHER_CANCEL,
workflow.TxMetaType.ETHER_SPEEDUP,
];

// nothing to estimate
if (gasIsDefined) {
return;
}

if (workflow.isEthereumPlainTx(creatingTx) && workflow.isEthereumBasicPlainTx(creatingTx)) {
let builtTx: EthereumTransaction | undefined = workflow
.fromEthereumPlainTx(creatingTx, new TokenRegistry(getTokens(getState())))
.build();

if (!builtTx) {
return;
}

let estimateTx: EthereumBasicTransaction = {
from: builtTx.from,
to: builtTx.to,
data: builtTx.data,
value: new Wei(builtTx.value).toHex(),
}

if (estimateTx.to == null && estimateTx.data == null) {
// tx is not filled yet with anything
return;
}

return extra.backendApi
.estimateGasLimit(creatingTx.blockchain, estimateTx)
.then((gasLimit) => {
let action: SetGasLimitAction = {
type: ActionTypes.SET_GAS_LIMIT,
payload: gasLimit
}
dispatch(action);
})
.catch((error) => {
console.error('Failed to estimate gas limit', error);
});
}

}
}
20 changes: 20 additions & 0 deletions packages/store/src/txstash/reducer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
SetEntryAction,
SetFeeLoadingAction,
SetFeeRangeAction,
SetGasLimitAction,
SetPreparingAction,
SetSignedAction,
SetStageAction,
Expand Down Expand Up @@ -88,6 +89,23 @@ function setTransactionFee(
return { ...state, transactionFee };
}

function setGasLimit(state: TxStashState, { payload }: SetGasLimitAction): TxStashState {
if (state.stage === CreateTxStage.SETUP) {
let prev = state.transaction;
if (!prev) {
return state;
}
return {
...state,
transaction: {
...prev,
gas: payload,
}
};
}
return state;
}

export function reducer(state = INITIAL_STATE, action: TxStashAction): TxStashState {
switch (action.type) {
case ActionTypes.RESET:
Expand All @@ -112,6 +130,8 @@ export function reducer(state = INITIAL_STATE, action: TxStashAction): TxStashSt
return setTransaction(state, action);
case ActionTypes.SET_TRANSACTION_FEE:
return setTransactionFee(state, action);
case ActionTypes.SET_GAS_LIMIT:
return setGasLimit(state, action);
default:
return state;
}
Expand Down
10 changes: 9 additions & 1 deletion packages/store/src/txstash/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ export enum ActionTypes {
SET_STAGE = 'TX_STASH/SET_STAGE',
SET_TRANSACTION = 'TX_STASH/SET_TRANSACTION',
SET_TRANSACTION_FEE = 'TX_STASH/SET_TRANSACTION_FEE',
SET_GAS_LIMIT = 'TX_STASH/SET_GAS_LIMIT',
}

export interface ResetAction {
Expand Down Expand Up @@ -136,6 +137,11 @@ export interface SetTransactionFeeAction {
};
}

export interface SetGasLimitAction {
type: ActionTypes.SET_GAS_LIMIT;
payload: number;
}

export type TxStashAction =
| ResetAction
| SetAssetAction
Expand All @@ -147,7 +153,9 @@ export type TxStashAction =
| SetSignedAction
| SetStageAction
| SetTransactionAction
| SetTransactionFeeAction;
| SetTransactionFeeAction
| SetGasLimitAction
;

export interface TxOrigin {
action: TxAction;
Expand Down

0 comments on commit 9bef05b

Please sign in to comment.