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

perf: prevent non-gas-consuming calls invoke gas translators #11670

Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ export function SwitchChainRequest(props: InteractionItemProps) {

await Message!.approveRequestWithResult(props.currentRequest.ID, { result: null, jsonrpc: '2.0', id: 0 })
// After a chain switch, old requests should be dropped according to https://eips.ethereum.org/EIPS/eip-3326
await Message!.denyRequests({ keepChainUnrelated: true, keepNonceUnrelated: true })
await Message!.rejectRequests({ keepChainUnrelated: true, keepNonceUnrelated: true })
})

return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ export function TransactionRequest(props: InteractionItemProps) {
}),
)

const response = await Message?.approveRequest(request.ID, {
const response = await Message!.approveAndSendRequest(request.ID, {
arguments: {
...request.request.arguments,
params,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export function WalletSignRequest(props: InteractionItemProps) {
}
}

const response = await Message!.approveRequest(id, {
const response = await Message!.approveAndSendRequest(id, {
arguments: {
method: request.method,
params,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ export const WatchTokenRequest = memo<InteractionItemProps>((props) => {
})
}
// It is "deny" because we don't want send it to the upstream RPC.
await Message!.denyRequest(request.ID)
await Message!.rejectRequest(request.ID)
})

const { chainId } = useChainContext<NetworkPluginID.PLUGIN_EVM>()
Expand Down
4 changes: 2 additions & 2 deletions packages/mask/popups/pages/Wallet/Interaction/interaction.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ export const Interaction = memo((props: InteractionProps) => {
const confirmAction = useRef<(lastRequest: boolean) => Promise<void>>(async () => {})

const [{ loading: cancelLoading }, onCancel] = useAsyncFn(async () => {
await Message!.denyRequest(currentRequest.ID)
await Message!.rejectRequest(currentRequest.ID)
if (currentRequest.origin) await Services.Helper.removePopupWindow()
navigate(PopupRoutes.Wallet, { replace: true })
}, [currentRequest.ID, Message, currentRequest.origin])
Expand Down Expand Up @@ -177,7 +177,7 @@ const Pager = memo((props: InteractionProps) => {
const navigate = useNavigate()
const { Message } = useWeb3State(NetworkPluginID.PLUGIN_EVM)
const [{ loading: cancelAllLoading }, handleCancelAllRequest] = useAsyncFn(async () => {
await Message!.denyRequests({ keepChainUnrelated: false, keepNonceUnrelated: false })
await Message!.rejectRequests({ keepChainUnrelated: false, keepNonceUnrelated: false })
if (currentRequest.origin) await Services.Helper.removePopupWindow()
else navigate(PopupRoutes.Wallet, { replace: true })
}, [Message, currentRequest.origin])
Expand Down
39 changes: 13 additions & 26 deletions packages/web3-providers/src/Web3/Base/state/Message.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,38 +30,24 @@ export abstract class MessageState<Request extends object, Response extends obje
return message
}

protected async validateMessage(message: TransferableMessage<Request, Response>) {
return true
}

protected async waitForApprovingRequest(id: string): Promise<ReasonableMessage<Request, Response>> {
protected waitForApprovingRequest(id: string): Promise<ReasonableMessage<Request, Response>> {
return new Promise((resolve, reject) => {
const observe = () => {
const message = this.storage.value[id]

if (message) {
// not a state to be resolved
if (message.state === MessageStateType.NOT_DEPEND) return

if (message.state === MessageStateType.APPROVED) resolve(message)
else reject(new Error('User rejected the message.'))
} else {
reject(new Error('Invalid request ID'))
}

const message = this.assertMessage(id)
// not a state to be resolved
if (message.state === MessageStateType.NOT_DEPEND) return
if (message.state === MessageStateType.APPROVED) resolve(message)
else reject(new Error('User rejected the message.'))
unsubscribe()
}

const unsubscribe = this.storage.subscription.subscribe(observe)
observe()
})
}

private async applyRequest(
private async createRequest(
message: TransferableMessage<Request, Response>,
): Promise<ReasonableMessage<Request, Response>> {
await this.validateMessage(message)

const ID = uuid()
const now = new Date()
const message_ = {
Expand All @@ -80,14 +66,15 @@ export abstract class MessageState<Request extends object, Response extends obje
}
draft[ID] = message_
})
console.log(nextMessages)
await this.storage.setValue(nextMessages)
return message_
}

async applyAndWaitResponse(
async createRequestAndWaitForApproval(
message: TransferableMessage<Request, Response>,
): Promise<ReasonableMessage<Request, Response>> {
const { ID } = await this.applyRequest(message)
const { ID } = await this.createRequest(message)
const reasonableMessage = await this.waitForApprovingRequest(ID)
if (!reasonableMessage.response) throw new Error('Invalid response')
return reasonableMessage
Expand All @@ -104,7 +91,7 @@ export abstract class MessageState<Request extends object, Response extends obje
)
}

abstract approveRequest(id: string, updates?: Request): Promise<Response | void>
abstract approveAndSendRequest(id: string, updates?: Request): Promise<Response | void>

async approveRequestWithResult(id: string, result: Response): Promise<void> {
await this.updateMessage(id, {
Expand All @@ -113,13 +100,13 @@ export abstract class MessageState<Request extends object, Response extends obje
})
}

async denyRequest(id: string): Promise<void> {
async rejectRequest(id: string): Promise<void> {
await this.updateMessage(id, {
state: MessageStateType.DENIED,
})
}

async denyRequests({ keepChainUnrelated, keepNonceUnrelated }: DenyRequestOptions): Promise<void> {
async rejectRequests({ keepChainUnrelated, keepNonceUnrelated }: DenyRequestOptions): Promise<void> {
const messages = produce(this.storage.value, (draft: typeof this.storage.value) => {
for (const key in draft) {
if (draft[key].state === MessageStateType.NOT_DEPEND) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ export class Popups implements Middleware<ConnectionContext> {
},
},
}
const { request: updates, response } = await evm.state.Message.applyAndWaitResponse(request)
const { request: updates, response } = await evm.state.Message.createRequestAndWaitForApproval(request)

context.config = {
...context.config,
Expand Down
6 changes: 3 additions & 3 deletions packages/web3-providers/src/Web3/EVM/state/Message.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ export class EVMMessage extends MessageState<MessageRequest, MessageResponse> {
const { request } = this.assertMessage(id)

if (request.options.silent) {
await this.approveRequest(id)
await this.approveAndSendRequest(id)
} else {
// TODO: make this for Mask Wallet only
const hasPassword = await this.context.hasPaymentPassword()
Expand All @@ -101,7 +101,7 @@ export class EVMMessage extends MessageState<MessageRequest, MessageResponse> {
return super.waitForApprovingRequest(id)
}

async approveRequest(id: string, updates?: MessageRequest): Promise<JsonRpcResponse | void> {
async approveAndSendRequest(id: string, updates?: MessageRequest): Promise<JsonRpcResponse | void> {
const { request: request_ } = this.assertMessage(id)

const request = await this.updateRequest(request_, updates)
Expand All @@ -119,7 +119,7 @@ export class EVMMessage extends MessageState<MessageRequest, MessageResponse> {
})

// deny all requests after approving one
await this.denyRequests({
await this.rejectRequests({
keepChainUnrelated: this.isChainUnrelated(request),
keepNonceUnrelated: this.isNonceUnrelated(request),
})
Expand Down
2 changes: 1 addition & 1 deletion packages/web3-providers/src/Web3/EVM/translators/Base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { EVMChainResolver } from '../apis/ResolverAPI.js'
export abstract class BaseTranslator implements Translator<ConnectionContext> {
async encode(context: ConnectionContext) {
const config = context.config
if (!config || PayloadEditor.fromPayload(context.request).readonly) return
if (!config || !PayloadEditor.fromPayload(context.request).gasConsuming) return

// #region polyfill transaction config
try {
Expand Down
20 changes: 11 additions & 9 deletions packages/web3-shared/base/src/specs/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1027,16 +1027,18 @@ export interface MessageState<Request, Response> {
messages: Subscription<Array<ReasonableMessage<Request, Response>>>
/** Updates a request. */
updateMessage(id: string, updates: Partial<TransferableMessage<Request, Response>>): Promise<void>
/** Applies a request and waits for confirmation from the user. */
applyAndWaitResponse(message: TransferableMessage<Request, Response>): Promise<ReasonableMessage<Request, Response>>
/** Approves a request. */
approveRequest(id: string, updates?: Request): Promise<Response | void>
/** Approves and resolve a request without sending them to the network. */
/** Create a request and waits for approval from the user. */
createRequestAndWaitForApproval(
message: TransferableMessage<Request, Response>,
): Promise<ReasonableMessage<Request, Response>>
/** Approve a request and send it to the network (usually transactions). */
approveAndSendRequest(id: string, updates?: Request): Promise<Response | void>
/** Approve and resolve a request with the given result. */
approveRequestWithResult(id: string, result: Response): Promise<void>
/** Rejects a request. */
denyRequest(id: string): Promise<void>
/** Rejects requests. */
denyRequests(options: DenyRequestOptions): Promise<void>
/** Reject a request. */
rejectRequest(id: string): Promise<void>
/** Reject requests. */
rejectRequests(options: DenyRequestOptions): Promise<void>
}

/** If you set both value */
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { EthereumMethodType } from '../types/index.js'

export const gasConsumingMethodType = [
Copy link
Member Author

Choose a reason for hiding this comment

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

cc @guanbinrui is this list ok?

EthereumMethodType.eth_sendTransaction,
EthereumMethodType.eth_signTransaction,
EthereumMethodType.MASK_REPLACE_TRANSACTION,
] as const
Object.freeze(gasConsumingMethodType)
18 changes: 2 additions & 16 deletions packages/web3-shared/evm/src/helpers/isMaskOnlyMethodType.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,5 @@
import { EthereumMethodType } from '../types/index.js'
import { type EthereumMethodType } from '../types/index.js'

export function isMaskOnlyMethodType(type: EthereumMethodType) {
return [
EthereumMethodType.MASK_DEPLOY,
EthereumMethodType.MASK_FUND,
EthereumMethodType.MASK_LOGIN,
EthereumMethodType.MASK_LOGOUT,
EthereumMethodType.MASK_WALLETS,
EthereumMethodType.MASK_ADD_WALLET,
EthereumMethodType.MASK_UPDATE_WALLET,
EthereumMethodType.MASK_RENAME_WALLET,
EthereumMethodType.MASK_REMOVE_WALLET,
EthereumMethodType.MASK_UPDATE_WALLETS,
EthereumMethodType.MASK_REMOVE_WALLETS,
EthereumMethodType.MASK_RESET_ALL_WALLETS,
EthereumMethodType.MASK_REPLACE_TRANSACTION,
].includes(type)
return type.startsWith('MASK_') || type.startsWith('mask_')
}
27 changes: 13 additions & 14 deletions packages/web3-shared/evm/src/helpers/isRiskyMethodType.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,15 @@
import { EthereumMethodType } from '../types/index.js'

export function isRiskyMethodType(type: EthereumMethodType) {
return [
EthereumMethodType.eth_sign,
EthereumMethodType.personal_sign,
EthereumMethodType.wallet_watchAsset,
EthereumMethodType.wallet_requestPermissions,
EthereumMethodType.wallet_addEthereumChain,
EthereumMethodType.wallet_switchEthereumChain,
EthereumMethodType.eth_signTypedData_v4,
EthereumMethodType.eth_sendTransaction,
EthereumMethodType.eth_signTransaction,
EthereumMethodType.MASK_REPLACE_TRANSACTION,
].includes(type as EthereumMethodType)
}
export const riskyMethodType = [
EthereumMethodType.eth_sign,
EthereumMethodType.personal_sign,
EthereumMethodType.wallet_watchAsset,
EthereumMethodType.wallet_requestPermissions,
EthereumMethodType.wallet_addEthereumChain,
EthereumMethodType.wallet_switchEthereumChain,
EthereumMethodType.eth_signTypedData_v4,
EthereumMethodType.eth_sendTransaction,
EthereumMethodType.eth_signTransaction,
EthereumMethodType.MASK_REPLACE_TRANSACTION,
] as const
Object.freeze(riskyMethodType)
17 changes: 9 additions & 8 deletions packages/web3-shared/evm/src/libs/PayloadEditor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,9 @@ import {
EthereumMethodType,
} from '../types/index.js'
import { abiCoder } from '../helpers/abiCoder.js'
import { isReadonlyMethodType } from '../helpers/isReadonlyMethodType.js'
import { isRiskyMethodType } from '../helpers/isRiskyMethodType.js'
import { readonlyMethodType } from '../helpers/isReadonlyMethodType.js'
import { riskyMethodType } from '../helpers/isRiskyMethodType.js'
import { gasConsumingMethodType } from '../helpers/isGasConsumingMethodType.js'

type Options = Pick<TransactionOptions, 'account' | 'chainId'>

Expand All @@ -37,10 +38,6 @@ export class PayloadEditor {
return this.payload.method
}

get params() {
return this.payload.params ?? []
}

get from(): string | undefined {
const { method, params } = this.payload
switch (method) {
Expand Down Expand Up @@ -231,11 +228,15 @@ export class PayloadEditor {
}

get risky() {
return isRiskyMethodType(this.payload.method as EthereumMethodType)
return (riskyMethodType as readonly string[]).includes(this.payload.method)
}

get readonly() {
return isReadonlyMethodType(this.payload.method as EthereumMethodType)
return (readonlyMethodType as readonly string[]).includes(this.payload.method)
}

get gasConsuming() {
return (gasConsumingMethodType as readonly string[]).includes(this.payload.method)
}

fill() {
Expand Down
Loading