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

feat: add more bridge fields to txMeta for Bridge #4918

Closed
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ import { getSimulationData } from './utils/simulation';
import {
updatePostTransactionBalance,
updateSwapsTransaction,
updateBridgeTypesTransaction,
} from './utils/swaps';

type UnrestrictedControllerMessenger = ControllerMessenger<
Expand Down Expand Up @@ -477,6 +478,9 @@ describe('TransactionController', () => {
const estimateGasMock = jest.mocked(estimateGas);
const addGasBufferMock = jest.mocked(addGasBuffer);
const updateSwapsTransactionMock = jest.mocked(updateSwapsTransaction);
const updateBridgeTypesTransactionMock = jest.mocked(
updateBridgeTypesTransaction,
);
const updatePostTransactionBalanceMock = jest.mocked(
updatePostTransactionBalance,
);
Expand Down Expand Up @@ -622,6 +626,7 @@ describe('TransactionController', () => {
disableHistory: false,
disableSendFlowHistory: false,
disableSwaps: false,
disableBridge: false,
getCurrentNetworkEIP1559Compatibility: async () => false,
getNetworkState: () => networkState,
// TODO: Replace with a real type
Expand Down Expand Up @@ -875,6 +880,9 @@ describe('TransactionController', () => {
updateSwapsTransactionMock.mockImplementation(
(transactionMeta) => transactionMeta,
);
updateBridgeTypesTransactionMock.mockImplementation(
(transactionMeta) => transactionMeta,
);

getAccountAddressRelationshipMock.mockResolvedValue({
count: 1,
Expand Down Expand Up @@ -1430,6 +1438,7 @@ describe('TransactionController', () => {
const transactionMeta = controller.state.transactions[0];

expect(updateSwapsTransactionMock).toHaveBeenCalledTimes(1);
expect(updateBridgeTypesTransactionMock).toHaveBeenCalledTimes(1);
expect(transactionMeta.txParams.from).toBe(ACCOUNT_MOCK);
expect(transactionMeta.chainId).toBe(MOCK_NETWORK.chainId);
expect(transactionMeta.deviceConfirmedOn).toBe(mockDeviceConfirmedOn);
Expand Down
37 changes: 37 additions & 0 deletions packages/transaction-controller/src/TransactionController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ import { hasSimulationDataChanged, shouldResimulate } from './utils/resimulate';
import { getTransactionParamsWithIncreasedGasFee } from './utils/retry';
import { getSimulationData } from './utils/simulation';
import {
updateBridgeTypesTransaction,
updatePostTransactionBalance,
updateSwapsTransaction,
} from './utils/swaps';
Expand Down Expand Up @@ -251,6 +252,7 @@ export type PendingTransactionOptions = {
* @property disableHistory - Whether to disable storing history in transaction metadata.
* @property disableSendFlowHistory - Explicitly disable transaction metadata history.
* @property disableSwaps - Whether to disable additional processing on swaps transactions.
* @property disableBridge - Whether to disable additional processing on bridge transactions.
* @property getCurrentAccountEIP1559Compatibility - Whether or not the account supports EIP-1559.
* @property getCurrentNetworkEIP1559Compatibility - Whether or not the network supports EIP-1559.
* @property getExternalPendingTransactions - Callback to retrieve pending transactions from external sources.
Expand Down Expand Up @@ -284,6 +286,7 @@ export type TransactionControllerOptions = {
disableHistory: boolean;
disableSendFlowHistory: boolean;
disableSwaps: boolean;
disableBridge: boolean;
getCurrentAccountEIP1559Compatibility?: () => Promise<boolean>;
getCurrentNetworkEIP1559Compatibility: () => Promise<boolean>;
getExternalPendingTransactions?: (
Expand Down Expand Up @@ -467,6 +470,22 @@ export type TransactionControllerTransactionNewSwapAndSendEvent = {
payload: [{ transactionMeta: TransactionMeta }];
};

/**
* Represents the `TransactionController:transactionNewBridgeApproval` event.
*/
export type TransactionControllerTransactionNewBridgeApprovalEvent = {
Copy link
Member

Choose a reason for hiding this comment

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

In an ideal world, the controller wouldn't be coupled to specific transaction types in the events since we already have the unapprovedTransactionAdded.

Could we instead just subscribe to that in the client and check the type property?

Copy link
Contributor Author

@infiniteflower infiniteflower Nov 25, 2024

Choose a reason for hiding this comment

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

Yes we can, I was just referencing the similar Swap types here initially.

type: `${typeof controllerName}:transactionNewBridgeApproval`;
payload: [{ transactionMeta: TransactionMeta }];
};

/**
* Represents the `TransactionController:transactionNewBridge` event.
*/
export type TransactionControllerTransactionNewBridgeEvent = {
type: `${typeof controllerName}:transactionNewBridge`;
payload: [{ transactionMeta: TransactionMeta }];
};

/**
* Represents the `TransactionController:transactionPublishingSkipped` event.
*/
Expand Down Expand Up @@ -537,6 +556,8 @@ export type TransactionControllerEvents =
| TransactionControllerTransactionNewSwapApprovalEvent
| TransactionControllerTransactionNewSwapEvent
| TransactionControllerTransactionNewSwapAndSendEvent
| TransactionControllerTransactionNewBridgeApprovalEvent
| TransactionControllerTransactionNewBridgeEvent
| TransactionControllerTransactionPublishingSkipped
| TransactionControllerTransactionRejectedEvent
| TransactionControllerTransactionStatusUpdatedEvent
Expand Down Expand Up @@ -591,6 +612,8 @@ export class TransactionController extends BaseController<

private readonly isSwapsDisabled: boolean;

private readonly isBridgeDisabled: boolean;

private readonly isSendFlowHistoryDisabled: boolean;

private readonly approvingTransactionIds: Set<string> = new Set();
Expand Down Expand Up @@ -753,6 +776,7 @@ export class TransactionController extends BaseController<
* @param options.blockTracker - The block tracker used to poll for new blocks data.
* @param options.disableHistory - Whether to disable storing history in transaction metadata.
* @param options.disableSendFlowHistory - Explicitly disable transaction metadata history.
* @param options.disableBridge - Whether to disable additional processing on bridge transactions.
* @param options.disableSwaps - Whether to disable additional processing on swaps transactions.
* @param options.getCurrentAccountEIP1559Compatibility - Whether or not the account supports EIP-1559.
* @param options.getCurrentNetworkEIP1559Compatibility - Whether or not the network supports EIP-1559.
Expand Down Expand Up @@ -783,6 +807,7 @@ export class TransactionController extends BaseController<
disableHistory,
disableSendFlowHistory,
disableSwaps,
disableBridge,
getCurrentAccountEIP1559Compatibility,
getCurrentNetworkEIP1559Compatibility,
getExternalPendingTransactions,
Expand Down Expand Up @@ -824,6 +849,7 @@ export class TransactionController extends BaseController<
this.isSwapsDisabled = disableSwaps ?? false;
this.#isFirstTimeInteractionEnabled =
isFirstTimeInteractionEnabled ?? (() => true);
this.isBridgeDisabled = disableBridge ?? false;
this.#isSimulationEnabled = isSimulationEnabled ?? (() => true);
// @ts-expect-error the type in eth-method-registry is inappropriate and should be changed
this.registry = new MethodRegistry({ provider });
Expand Down Expand Up @@ -1153,6 +1179,17 @@ export class TransactionController extends BaseController<
},
);

addedTransactionMeta = updateBridgeTypesTransaction(
infiniteflower marked this conversation as resolved.
Show resolved Hide resolved
addedTransactionMeta,
transactionType,
swaps,
{
isBridgeDisabled: this.isBridgeDisabled,
cancelTransaction: this.cancelTransaction.bind(this),
messenger: this.messagingSystem,
},
);

this.addMetadata(addedTransactionMeta);

if (requireApproval !== false) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,7 @@ const setupController = async (
disableHistory: false,
disableSendFlowHistory: false,
disableSwaps: false,
disableBridge: false,
getCurrentNetworkEIP1559Compatibility: async (
networkClientId?: NetworkClientId,
) => {
Expand Down
44 changes: 44 additions & 0 deletions packages/transaction-controller/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,11 @@ type TransactionMetaBase = {
*/
blockTimestamp?: string;

/**
*
*/
bridgeSteps?: BridgeStep[];
Copy link
Member

Choose a reason for hiding this comment

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

To clarify, do we definitely need to store this here? We don't have a bridge specific controller that will persist this state also and can link to a transaction ID?


/**
* Network code as per EIP-155 for this transaction.
*/
Expand Down Expand Up @@ -121,6 +126,11 @@ type TransactionMetaBase = {
*/
deviceConfirmedOn?: WalletDevice;

/**
* The Network ID as per EIP-155 of the destination chain of a bridge transaction.
*/
destinationChainId?: Hex;

/**
* The address of the token being received of swap transaction.
*/
Expand Down Expand Up @@ -1354,3 +1364,37 @@ export type SubmitHistoryEntry = {
export type InternalAccount = ReturnType<
AccountsController['getSelectedAccount']
>;
enum BridgeActionTypes {
Copy link
Member

Choose a reason for hiding this comment

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

Minor, line break.

BRIDGE = 'bridge',
SWAP = 'swap',
REFUEL = 'refuel',
}

type BridgeProtocol = {
name: string;
displayName?: string;
icon?: string;
};

/**
* A step in a bridge transaction.
*/
export type BridgeStep = {
action: BridgeActionTypes;
srcChainId: Hex;
destChainId?: Hex;
srcAsset: BridgeAsset;
destAsset: BridgeAsset;
srcAmount: string;
destAmount: string;
protocol: BridgeProtocol;
};

export type BridgeAsset = {
Copy link
Member

Choose a reason for hiding this comment

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

JSDoc for types and all properties?

chainId: Hex;
address: string;
symbol: string;
name: string;
decimals: number;
icon?: string;
};
10 changes: 6 additions & 4 deletions packages/transaction-controller/src/utils/gas-fees.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import type {
} from '../types';
import { GasFeeEstimateType, UserFeeLevel } from '../types';
import { getGasFeeFlow } from './gas-flow';
import { SWAP_TRANSACTION_TYPES } from './swaps';
import { BRIDGE_TRANSACTION_TYPES, SWAP_TRANSACTION_TYPES } from './swaps';

export type UpdateGasFeesRequest = {
eip1559: boolean;
Expand Down Expand Up @@ -58,9 +58,11 @@ export async function updateGasFees(request: UpdateGasFeesRequest) {
const isSwap = SWAP_TRANSACTION_TYPES.includes(
txMeta.type as TransactionType,
);
const savedGasFees = isSwap
? undefined
: request.getSavedGasFees(txMeta.chainId);
const isBridge = BRIDGE_TRANSACTION_TYPES.includes(
txMeta.type as TransactionType,
);
const savedGasFees =
isSwap || isBridge ? undefined : request.getSavedGasFees(txMeta.chainId);

const suggestedGasFees = await getSuggestedGasFees(request);

Expand Down
Loading
Loading