Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 1
feat: support args.receiverAddress override + allow zero ERC-20 approvals#19
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
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Uh oh!
There was an error while loading. Please reload this page.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -8,6 +8,7 @@ | ||
| import { BaseEVMValidator, EVMTransaction } from '../base.validator'; | ||
| import { VaultInfo, VaultConfiguration } from './types'; | ||
| import { WETH_ADDRESSES } from './constants'; | ||
| import { isNonEmptyString } from '../../../utils/validation'; | ||
| /** | ||
| * Standard ERC4626 ABI - only the functions we need to validate | ||
| @@ -98,7 +99,7 @@ | ||
| unsignedTransaction: string, | ||
| transactionType: TransactionType, | ||
| userAddress: string, | ||
| _args?: ActionArguments, | ||
| args?: ActionArguments, | ||
| _context?: ValidationContext, | ||
| ): ValidationResult { | ||
| const decoded = this.decodeEVMTransaction(unsignedTransaction); | ||
| @@ -116,25 +117,29 @@ | ||
| // Get and validate chain ID from transaction | ||
| const chainId = this.getNumericChainId(tx); | ||
| if (!chainId) { | ||
Check warning on line 120 in src/validators/evm/erc4626/erc4626.validator.ts
| ||
| return this.blocked('Chain ID not found in transaction'); | ||
| } | ||
| // Ensure destination address exists | ||
| if (!tx.to) { | ||
Check warning on line 125 in src/validators/evm/erc4626/erc4626.validator.ts
| ||
| return this.blocked('Transaction has no destination address'); | ||
| } | ||
| const receiverAddress = isNonEmptyString(args?.receiverAddress) | ||
| ? args.receiverAddress | ||
| : undefined; | ||
coderabbitai[bot] marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| // Route to appropriate validation based on transaction type | ||
| switch (transactionType) { | ||
| case TransactionType.APPROVAL: | ||
| return this.validateApproval(tx, chainId); | ||
| case TransactionType.WRAP: | ||
| return this.validateWrap(tx, chainId); | ||
| case TransactionType.SUPPLY: | ||
| return this.validateSupply(tx, userAddress, chainId); | ||
| return this.validateSupply(tx, userAddress, chainId, receiverAddress); | ||
| case TransactionType.WITHDRAW: | ||
| return this.validateWithdraw(tx, userAddress, chainId); | ||
| return this.validateWithdraw(tx, userAddress, chainId, receiverAddress); | ||
| case TransactionType.UNWRAP: | ||
| return this.validateUnwrap(tx, chainId); | ||
| default: | ||
| @@ -177,7 +182,7 @@ | ||
| } | ||
| // Get spender (should be vault address) | ||
| const [spender, amount] = parsed.args; | ||
| const [spender] = parsed.args; | ||
| // Validate spender is a whitelisted vault | ||
| const vaultInfo = this.vaultInfoMap.get( | ||
| @@ -196,12 +201,6 @@ | ||
| }); | ||
| } | ||
| // Validate amount is not zero | ||
| const amountBigInt = BigInt(amount); | ||
| if (amountBigInt === 0n) { | ||
| return this.blocked('Approval amount is zero'); | ||
| } | ||
| return this.safe(); | ||
| } | ||
| @@ -211,7 +210,7 @@ | ||
| private validateWrap(tx: EVMTransaction, chainId: number): ValidationResult { | ||
| // Get WETH address for this chain | ||
| const wethAddress = this.getWethAddress(chainId); | ||
| if (!wethAddress) { | ||
Check warning on line 213 in src/validators/evm/erc4626/erc4626.validator.ts
| ||
| return this.blocked('WETH address not configured for chain', { chainId }); | ||
| } | ||
| @@ -265,6 +264,7 @@ | ||
| tx: EVMTransaction, | ||
| userAddress: string, | ||
| chainId: number, | ||
| receiverAddress?: string, | ||
| ): ValidationResult { | ||
| const resolved = this.resolveVault(tx, chainId); | ||
| if ('error' in resolved) return resolved.error; | ||
| @@ -310,10 +310,12 @@ | ||
| return this.blocked('Supply amount is zero'); | ||
| } | ||
| // Validate receiver is the user | ||
| if (receiver.toLowerCase() !== userAddress.toLowerCase()) { | ||
| return this.blocked('Receiver address does not match user address', { | ||
| expected: userAddress, | ||
| // Validate receiver is the intended receiver | ||
| const expectedReceiver = receiverAddress ?? userAddress; | ||
| if (receiver.toLowerCase() !== expectedReceiver.toLowerCase()) { | ||
| return this.blocked('Receiver address does not match expected address', { | ||
| expected: expectedReceiver, | ||
| actual: receiver, | ||
| }); | ||
| } | ||
| @@ -328,6 +330,7 @@ | ||
| tx: EVMTransaction, | ||
| userAddress: string, | ||
| chainId: number, | ||
| receiverAddress?: string, | ||
| ): ValidationResult { | ||
| const resolved = this.resolveVault(tx, chainId); | ||
| if ('error' in resolved) return resolved.error; | ||
| @@ -381,10 +384,11 @@ | ||
| }); | ||
| } | ||
| // Validate receiver is the user (for safety) | ||
| if (receiver.toLowerCase() !== userAddress.toLowerCase()) { | ||
| return this.blocked('Receiver address does not match user address', { | ||
| expected: userAddress, | ||
| const expectedReceiver = receiverAddress ?? userAddress; | ||
| if (receiver.toLowerCase() !== expectedReceiver.toLowerCase()) { | ||
| return this.blocked('Receiver address does not match expected address', { | ||
| expected: expectedReceiver, | ||
| actual: receiver, | ||
| }); | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agree with the bot, receiverAddress looks wired correctly e2e, one small follow-up might be adding a runtime type check here as well, since the schema only covers the JSON path and a truthy no string value could still reach .toLowerCase
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, now using isNonEmptyString(args?.receiverAddress) to guard the extraction, which matches the pattern used by the Tron and Solana validators for their args.validatorAddress access. An empty string or non-string value now normalizes to undefined, falling back to userAddress as expected.
Esp since this is a security library good to have clear errors instead of unhandled TypeErrors, thanks for flagging!