Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions packages/transaction-controller/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Added

- Add `isExternalSign` property to `TransactionMeta` to disable nonce generation and signing ([#5604](https://github.com/MetaMask/core/pull/5604))
- Add types for `isAtomicBatchSupported` method ([#5600](https://github.com/MetaMask/core/pull/5600))
- `IsAtomicBatchSupportedRequest`
- `IsAtomicBatchSupportedResult`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2452,6 +2452,45 @@ describe('TransactionController', () => {
expect(publishHook).toHaveBeenCalledTimes(1);
});

it('skips signing if isExternalSign is true', async () => {
const { controller, mockTransactionApprovalRequest } =
setupController();

const signSpy = jest.spyOn(controller, 'sign');

const { result, transactionMeta } = await controller.addTransaction(
{
from: ACCOUNT_MOCK,
gas: '0x0',
gasPrice: '0x0',
to: ACCOUNT_MOCK,
value: '0x0',
},
{
networkClientId: NETWORK_CLIENT_ID_MOCK,
},
);

mockTransactionApprovalRequest.approve({
value: {
txMeta: {
...transactionMeta,
isExternalSign: true,
},
},
});

await result;

expect(signSpy).not.toHaveBeenCalled();

expect(controller.state.transactions).toMatchObject([
expect.objectContaining({
status: TransactionStatus.submitted,
}),
]);
});

describe('fails', () => {
/**
* Test template to assert adding and submitting a transaction fails.
Expand Down
26 changes: 16 additions & 10 deletions packages/transaction-controller/src/TransactionController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1703,7 +1703,7 @@ export class TransactionController extends BaseController<
}

// Update same nonce local transactions as dropped and define replacedBy properties.
this.markNonceDuplicatesDropped(transactionId);
this.#markNonceDuplicatesDropped(transactionId);

// Update external provided transaction with updated gas values and confirmed status.
this.updateTransaction(
Expand Down Expand Up @@ -2878,7 +2878,7 @@ export class TransactionController extends BaseController<

const rawTx = await this.#trace(
{ name: 'Sign', parentContext: traceContext },
() => this.signTransaction(transactionMeta),
() => this.#signTransaction(transactionMeta),
);

if (!(await this.beforePublish(transactionMeta))) {
Expand All @@ -2890,7 +2890,7 @@ export class TransactionController extends BaseController<
return ApprovalState.SkippedViaBeforePublishHook;
}

if (!rawTx) {
if (!rawTx && !transactionMeta.isExternalSign) {
return ApprovalState.NotApproved;
}

Expand Down Expand Up @@ -2934,7 +2934,7 @@ export class TransactionController extends BaseController<

({ transactionHash: hash } = await publishHook(
transactionMeta,
rawTx,
rawTx ?? '0x',
));

if (hash === undefined) {
Expand Down Expand Up @@ -3358,7 +3358,7 @@ export class TransactionController extends BaseController<
*
* @param transactionId - Used to identify original transaction.
*/
private markNonceDuplicatesDropped(transactionId: string) {
#markNonceDuplicatesDropped(transactionId: string) {
const transactionMeta = this.getTransaction(transactionId);
if (!transactionMeta) {
return;
Expand All @@ -3371,6 +3371,7 @@ export class TransactionController extends BaseController<
(transaction) =>
transaction.id !== transactionId &&
transaction.txParams.from === from &&
nonce &&
transaction.txParams.nonce === nonce &&
transaction.chainId === chainId &&
transaction.type !== TransactionType.incoming,
Expand Down Expand Up @@ -3483,10 +3484,15 @@ export class TransactionController extends BaseController<
);
}

private async signTransaction(
async #signTransaction(
transactionMeta: TransactionMeta,
): Promise<string | undefined> {
const { txParams } = transactionMeta;
const { isExternalSign, txParams } = transactionMeta;

if (isExternalSign) {
log('Skipping sign as signed externally');
return undefined;
}

log('Signing transaction', txParams);

Expand Down Expand Up @@ -3583,10 +3589,10 @@ export class TransactionController extends BaseController<
);
}

private onConfirmedTransaction(transactionMeta: TransactionMeta) {
#onConfirmedTransaction(transactionMeta: TransactionMeta) {
log('Processing confirmed transaction', transactionMeta.id);

this.markNonceDuplicatesDropped(transactionMeta.id);
this.#markNonceDuplicatesDropped(transactionMeta.id);

this.messagingSystem.publish(
`${controllerName}:transactionConfirmed`,
Expand Down Expand Up @@ -3730,7 +3736,7 @@ export class TransactionController extends BaseController<
) {
pendingTransactionTracker.hub.on(
'transaction-confirmed',
this.onConfirmedTransaction.bind(this),
this.#onConfirmedTransaction.bind(this),
);

pendingTransactionTracker.hub.on(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -526,6 +526,45 @@ describe('PendingTransactionTracker', () => {

expect(listener).not.toHaveBeenCalled();
});

it('unless no nonce', async () => {
const listener = jest.fn();

const confirmedTransactionMetaMock = {
...TRANSACTION_SUBMITTED_MOCK,
id: `${ID_MOCK}2`,
status: TransactionStatus.confirmed,
txParams: {
...TRANSACTION_SUBMITTED_MOCK.txParams,
nonce: undefined,
},
} as unknown as TransactionMeta;

const submittedTransactionMetaMock = {
...TRANSACTION_SUBMITTED_MOCK,
txParams: {
...TRANSACTION_SUBMITTED_MOCK.txParams,
nonce: undefined,
},
};

pendingTransactionTracker = new PendingTransactionTracker({
...options,
getTransactions: () => [
confirmedTransactionMetaMock,
submittedTransactionMetaMock,
],
});

pendingTransactionTracker.hub.addListener(
'transaction-dropped',
listener,
);

await onPoll();

expect(listener).not.toHaveBeenCalled();
});
});

describe('fires confirmed event', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -498,6 +498,7 @@ export class PendingTransactionTracker {
tx.id !== id &&
tx.txParams.from === txParams.from &&
tx.status === TransactionStatus.confirmed &&
tx.txParams.nonce &&
tx.txParams.nonce === txParams.nonce &&
tx.type !== TransactionType.incoming,
);
Expand Down
6 changes: 6 additions & 0 deletions packages/transaction-controller/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,12 @@ export type TransactionMeta = {
*/
id: string;

/**
* Whether the transaction is signed externally.
* No signing will be performed in the client and the `nonce` will be `undefined`.
*/
isExternalSign?: boolean;

/**
* Whether the transaction is a transfer.
*/
Expand Down
15 changes: 15 additions & 0 deletions packages/transaction-controller/src/utils/nonce.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,21 @@ describe('nonce', () => {

expect(releaseLock).toHaveBeenCalledTimes(1);
});

it('returns undefined if transaction is signed externally', async () => {
const transactionMeta = {
...TRANSACTION_META_MOCK,
isExternalSign: true,
};

const [nonce, releaseLock] = await getNextNonce(
transactionMeta,
jest.fn(),
);

expect(nonce).toBeUndefined();
expect(releaseLock).toBeUndefined();
});
});

describe('getAndFormatTransactionsForNonceTracker', () => {
Expand Down
7 changes: 6 additions & 1 deletion packages/transaction-controller/src/utils/nonce.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,17 @@ const log = createModuleLogger(projectLogger, 'nonce');
export async function getNextNonce(
txMeta: TransactionMeta,
getNonceLock: (address: string) => Promise<NonceLock>,
): Promise<[string, (() => void) | undefined]> {
): Promise<[string | undefined, (() => void) | undefined]> {
const {
customNonceValue,
isExternalSign,
txParams: { from, nonce: existingNonce },
} = txMeta;

if (isExternalSign) {
return [undefined, undefined];
}

const customNonce = customNonceValue ? toHex(customNonceValue) : undefined;

if (customNonce) {
Expand Down
Loading