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.

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

♻️ refactor(mm): Pass gas setting inside options object #1052

Merged
merged 1 commit into from
Jan 2, 2024
Merged

♻️ refactor(mm): Pass gas setting inside options object #1052

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
12 changes: 6 additions & 6 deletions wallets/metamask/src/metamask.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,12 +115,12 @@ export class MetaMask {
await this.notificationPage.rejectSwitchNetwork(this.extensionId)
}

async confirmTransaction(gasSetting: GasSetting = 'site') {
async confirmTransaction(options?: { gasSetting?: GasSetting }) {
if (!this.extensionId) {
throw NO_EXTENSION_ID_ERROR
}

await this.notificationPage.confirmTransaction(this.extensionId, gasSetting)
await this.notificationPage.confirmTransaction(this.extensionId, options)
}

async rejectTransaction() {
Expand Down Expand Up @@ -170,19 +170,19 @@ export class MetaMask {
// ---- EXPERIMENTAL FEATURES ----

public readonly experimental = {
confirmTransactionAndWaitForMining: async (gasSetting: GasSetting = 'site') =>
await this.confirmTransactionAndWaitForMining(gasSetting),
confirmTransactionAndWaitForMining: async (options?: { gasSetting?: GasSetting }) =>
await this.confirmTransactionAndWaitForMining(options),
// Note: `txIndex` starts from 0.
openTransactionDetails: async (txIndex: number) => await this.openTransactionDetails(txIndex),
closeTransactionDetails: async () => await this.closeTransactionDetails()
}

private async confirmTransactionAndWaitForMining(gasSetting: GasSetting) {
private async confirmTransactionAndWaitForMining(options?: { gasSetting?: GasSetting }) {
if (!this.extensionId) {
throw NO_EXTENSION_ID_ERROR
}

await this.notificationPage.confirmTransactionAndWaitForMining(this.extensionId, gasSetting)
await this.notificationPage.confirmTransactionAndWaitForMining(this.extensionId, options)
}

private async openTransactionDetails(txIndex: number) {
Expand Down
8 changes: 4 additions & 4 deletions wallets/metamask/src/pages/NotificationPage/page.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,10 @@ export class NotificationPage {
await network.rejectSwitchNetwork(notificationPage)
}

async confirmTransaction(extensionId: string, gasSetting: GasSetting) {
async confirmTransaction(extensionId: string, options?: { gasSetting?: GasSetting }) {
const notificationPage = await getNotificationPageAndWaitForLoad(this.page.context(), extensionId)

await transaction.confirm(notificationPage, gasSetting)
await transaction.confirm(notificationPage, options?.gasSetting ?? 'site')
}

async rejectTransaction(extensionId: string) {
Expand All @@ -102,10 +102,10 @@ export class NotificationPage {
await transaction.reject(notificationPage)
}

async confirmTransactionAndWaitForMining(extensionId: string, gasSetting: GasSetting) {
async confirmTransactionAndWaitForMining(extensionId: string, options?: { gasSetting?: GasSetting }) {
const notificationPage = await getNotificationPageAndWaitForLoad(this.page.context(), extensionId)

await transaction.confirmAndWaitForMining(this.page, notificationPage, gasSetting)
await transaction.confirmAndWaitForMining(this.page, notificationPage, options?.gasSetting ?? 'site')
}

async approvePermission(extensionId: string, options?: { spendLimit?: 'max' | number; gasSetting?: GasSetting }) {
Expand Down
42 changes: 26 additions & 16 deletions wallets/metamask/test/e2e/metamask/confirmTransaction.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ describe('with custom gas setting', () => {
}) => {
await connectAndTriggerEIP1559Transaction()

await expect(metamask.confirmTransaction('low')).rejects.toThrowError(
await expect(metamask.confirmTransaction({ gasSetting: 'low' })).rejects.toThrowError(
'[ConfirmTransaction] Estimated fee is not available for the "low" gas setting. By default, MetaMask would use the "site" gas setting in this case, however, this is not YOUR intention.'
)
})
Expand All @@ -60,7 +60,7 @@ describe('with custom gas setting', () => {
}) => {
await connectAndTriggerEIP1559Transaction()

await expect(metamask.confirmTransaction('market')).rejects.toThrowError(
await expect(metamask.confirmTransaction({ gasSetting: 'market' })).rejects.toThrowError(
'[ConfirmTransaction] Estimated fee is not available for the "market" gas setting. By default, MetaMask would use the "site" gas setting in this case, however, this is not YOUR intention.'
)
})
Expand All @@ -71,7 +71,7 @@ describe('with custom gas setting', () => {
}) => {
await connectAndTriggerEIP1559Transaction()

await expect(metamask.confirmTransaction('aggressive')).rejects.toThrowError(
await expect(metamask.confirmTransaction({ gasSetting: 'aggressive' })).rejects.toThrowError(
'[ConfirmTransaction] Estimated fee is not available for the "aggressive" gas setting. By default, MetaMask would use the "site" gas setting in this case, however, this is not YOUR intention.'
)
})
Expand All @@ -82,7 +82,7 @@ describe('with custom gas setting', () => {
}) => {
await connectAndTriggerEIP1559Transaction()

await metamask.confirmTransaction('site')
await metamask.confirmTransaction({ gasSetting: 'site' })
})

describe('with advanced (manual) gas setting', () => {
Expand All @@ -93,8 +93,10 @@ describe('with custom gas setting', () => {
await connectAndTriggerEIP1559Transaction()

const promise = metamask.confirmTransaction({
maxBaseFee: 250,
priorityFee: 300
gasSetting: {
maxBaseFee: 250,
priorityFee: 300
}
})

await expect(promise).rejects.toThrowError(
Expand All @@ -112,9 +114,11 @@ describe('with custom gas setting', () => {
await connectAndTriggerEIP1559Transaction()

const promise = metamask.confirmTransaction({
maxBaseFee: 250,
priorityFee: 150,
gasLimit: 10
gasSetting: {
maxBaseFee: 250,
priorityFee: 150,
gasLimit: 10
}
})

await expect(promise).rejects.toThrowError(
Expand All @@ -129,18 +133,22 @@ describe('with custom gas setting', () => {
await connectAndTriggerEIP1559Transaction()

await metamask.confirmTransaction({
maxBaseFee: 250,
priorityFee: 150,
gasLimit: 250_000
gasSetting: {
maxBaseFee: 250,
priorityFee: 150,
gasLimit: 250_000
}
})
})

test('should confirm transaction with small gas fee', async ({ metamask, connectAndTriggerEIP1559Transaction }) => {
await connectAndTriggerEIP1559Transaction()

await metamask.confirmTransaction({
maxBaseFee: 250,
priorityFee: 150
gasSetting: {
maxBaseFee: 250,
priorityFee: 150
}
})
})

Expand All @@ -149,8 +157,10 @@ describe('with custom gas setting', () => {
await connectAndTriggerEIP1559Transaction()

await metamask.confirmTransaction({
maxBaseFee: 250_000,
priorityFee: 150_000
gasSetting: {
maxBaseFee: 250_000,
priorityFee: 150_000
}
})
})
})
Expand Down