Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
8 Skipped Deployments
|
Deploying demo-ai-agents with
|
| Latest commit: |
0baf806
|
| Status: | 🚫 Build failed. |
There was a problem hiding this comment.
Pull request overview
This PR enhances the TON blockchain integration in the React wallet by adding comprehensive validation, improving message signing according to TON Connect specifications, and adding session properties for public keys and state initialization.
- Adds request validation with early error detection for TON transactions and messages
- Implements TON Connect compliant signing for authentication (ton-proof) and data signing with domain-aware hashing
- Introduces retry logic for network operations and comprehensive message validation
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| advanced/wallets/react-wallet-v2/package.json | Adds crc-32 dependency for TON signature verification |
| advanced/wallets/react-wallet-v2/pnpm-lock.yaml | Updates lock file with crc-32 package |
| advanced/wallets/react-wallet-v2/src/lib/TonLib.ts | Implements comprehensive validation, TON Connect compliant signing, retry logic, and new methods for public key/state init |
| advanced/wallets/react-wallet-v2/src/utils/TonRequestHandlerUtil.ts | Adds validateTonRequest function and updates approveTonRequest to accept session parameter for domain extraction |
| advanced/wallets/react-wallet-v2/src/views/SessionSignTonTransactionModal.tsx | Adds validation useEffect hook and passes session to approval handler |
| advanced/wallets/react-wallet-v2/src/views/SessionSignTonPersonalMessageModal.tsx | Adds validation useEffect hook and passes session to approval handler |
| advanced/wallets/react-wallet-v2/src/views/SessionProposalModal.tsx | Adds TON-specific session properties (public key and state init) |
| advanced/wallets/react-wallet-v2/src/utils/AuthUtil.ts | Implements TON-proof authentication using generateTonProof method with iat, domain, and payload |
Files not reviewed (1)
- advanced/wallets/react-wallet-v2/pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (4)
advanced/wallets/react-wallet-v2/src/views/SessionProposalModal.tsx:463
- Missing dependency in useCallback. The
authenticationMessagesToSignvariable is used within the callback on line 445, but it's not included in the dependency array on line 463. This could lead to stale closure issues where the callback uses an outdated value. AddauthenticationMessagesToSignto the dependency array.
const onApprove = useCallback(async () => {
console.log('onApprove', { proposal, namespaces })
try {
if (proposal && namespaces) {
setIsLoadingApprove(true)
if (reorderedEip155Accounts.length > 0) {
// we should append the smart accounts to the available eip155 accounts
namespaces.eip155.accounts = reorderedEip155Accounts.concat(namespaces.eip155.accounts)
}
//get capabilities for all reorderedEip155Accounts in wallet
const capabilities = getWalletCapabilities(reorderedEip155Accounts)
let sessionProperties = {
capabilities: JSON.stringify(capabilities)
} as any
// Add TRON-specific properties if TRON namespace exists
if (namespaces.tron) {
sessionProperties['tron_method_version'] = 'v1'
}
if (namespaces.bip122) {
const bip122Chain = namespaces.bip122.chains?.[0]!
sessionProperties.bip122_getAccountAddresses = JSON.stringify({
payment: Array.from(bip122Wallet.getAddresses(bip122Chain as IBip122ChainId).values()),
ordinal: Array.from(
bip122Wallet.getAddresses(bip122Chain as IBip122ChainId, ['ordinal']).values()
)
})
}
if (namespaces.sui) {
const suiWallet = await getSuiWallet()
const accounts = suiWallet.getAccounts()
sessionProperties.sui_getAccounts = JSON.stringify(accounts)
}
if (namespaces.stacks) {
const accounts = stacksWallet.getAccounts()
sessionProperties.stacks_getAddresses = JSON.stringify([
accounts.mainnet,
accounts.testnet
])
}
if (namespaces.ton) {
const tonWallet = await getWallet();
sessionProperties.ton_getPublicKey = tonWallet.getPublicKey();
sessionProperties.ton_getStateInit = tonWallet.getStateInit();
}
console.log('sessionProperties', sessionProperties)
const signedAuths = await signAuthenticationMessages(authenticationMessagesToSign)
await walletkit.approveSession({
id: proposal.id,
namespaces,
sessionProperties,
proposalRequestsResponses: {
authentication: signedAuths
}
})
SettingsStore.setSessions(Object.values(walletkit.getActiveSessions()))
}
} catch (e) {
styledToast((e as Error).message, 'error')
} finally {
setIsLoadingApprove(false)
ModalStore.close()
}
}, [namespaces, proposal, reorderedEip155Accounts])
advanced/wallets/react-wallet-v2/src/views/SessionSignTonTransactionModal.tsx:72
- Missing
requestSessionin the useCallback dependency array. The callback usesrequestSessionon line 60, but it's not included in the dependency array on line 72. This could lead to stale closure issues. AddrequestSessionto the dependency array.
const onApprove = useCallback(async () => {
try {
if (requestEvent) {
setIsLoadingApprove(true)
const response = await approveTonRequest(requestEvent, requestSession)
await walletkit.respondSessionRequest({
topic,
response
})
}
} catch (e) {
styledToast((e as Error).message, 'error')
} finally {
setIsLoadingApprove(false)
ModalStore.close()
}
}, [requestEvent, topic])
advanced/wallets/react-wallet-v2/src/views/SessionSignTonPersonalMessageModal.tsx:70
- Missing
requestSessionin the useCallback dependency array. The callback usesrequestSessionon line 58, but it's not included in the dependency array on line 70. This could lead to stale closure issues. AddrequestSessionto the dependency array.
const onApprove = useCallback(async () => {
try {
if (requestEvent) {
setIsLoadingApprove(true)
const response = await approveTonRequest(requestEvent, requestSession)
await walletkit.respondSessionRequest({
topic,
response
})
}
} catch (e) {
styledToast((e as Error).message, 'error')
} finally {
setIsLoadingApprove(false)
ModalStore.close()
}
}, [requestEvent, topic])
advanced/wallets/react-wallet-v2/src/lib/TonLib.ts:265
- Unused variable innerMessage.
const innerMessage = {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const domain = params.domain | ||
|
|
||
| const dataToSign = this.createTonProofMessageBytes({ | ||
| address: this.wallet.address, | ||
| domain: { | ||
| lengthBytes: domain.length, |
There was a problem hiding this comment.
Redundant variable assignment. The domain variable is assigned params.domain on line 307, but then params.domain is used directly on line 313. Either use domain consistently throughout the function or remove the redundant variable assignment.
| const domain = params.domain | |
| const dataToSign = this.createTonProofMessageBytes({ | |
| address: this.wallet.address, | |
| domain: { | |
| lengthBytes: domain.length, | |
| const dataToSign = this.createTonProofMessageBytes({ | |
| address: this.wallet.address, | |
| domain: { | |
| lengthBytes: params.domain.length, |
| private parseTonMessages(params: TonLib.SendMessage['params']) { | ||
| this.validateSendMessage(params) | ||
| return params.messages.map(m => { | ||
| const amountBigInt = typeof m.amount === 'string' ? BigInt(m.amount) : BigInt(m.amount) |
There was a problem hiding this comment.
Redundant type check and conversion. The condition typeof m.amount === 'string' ? BigInt(m.amount) : BigInt(m.amount) performs the same BigInt(m.amount) conversion in both branches. Since the type definition requires amount to be a string (line 511), this can be simplified to just BigInt(m.amount).
| const amountBigInt = typeof m.amount === 'string' ? BigInt(m.amount) : BigInt(m.amount) | |
| const amountBigInt = BigInt(m.amount) |
| domain: AuthMessage.domain, | ||
| payload: AuthMessage.statement, | ||
| }) | ||
| return { signature: tonResult.signature, publicKey: tonResult.publicKey, type: 'ton' } |
There was a problem hiding this comment.
Missing break statement after the ton case. This causes fall-through to the solana case, which will execute the solana logic when TON authentication doesn't have a statement. Add a break statement after line 141 or return early from the function.
| return { signature: tonResult.signature, publicKey: tonResult.publicKey, type: 'ton' } | |
| return { signature: tonResult.signature, publicKey: tonResult.publicKey, type: 'ton' } | |
| } else { | |
| throw new Error('TON authentication requires a statement'); |
| const payload = Array.isArray(request.params) ? request.params[0] : request.params || {} | ||
|
|
||
| useEffect(() => { | ||
| if (!requestEvent) { |
There was a problem hiding this comment.
Inconsistent validation check. In SessionSignTonTransactionModal.tsx line 38, the validation checks if (!request.params), but in SessionSignTonPersonalMessageModal.tsx line 36, it checks if (!requestEvent). These should be consistent. The check should likely be if (!request.params) in both files to validate that the request has parameters before calling validateTonRequest.
| if (!requestEvent) { | |
| if (!requestEvent || !requestEvent.params || !requestEvent.params.request) { |
| case TON_SIGNING_METHODS.SIGN_DATA: | ||
| break | ||
| case TON_SIGNING_METHODS.SEND_MESSAGE: | ||
| wallet.validateSendMessage(payload) |
There was a problem hiding this comment.
Missing break statement in switch case. The SEND_MESSAGE case should have a break statement to prevent fall-through to the default case, even though it's the last explicit case before default. This is a best practice for maintainability.
| wallet.validateSendMessage(payload) | |
| wallet.validateSendMessage(payload) | |
| break |
| * Creates hash for Cell payload according to TON Connect specification. | ||
| */ | ||
| /** |
There was a problem hiding this comment.
Duplicate documentation comment. There are two JSDoc comments on lines 327-329 and 330-334, both attempting to document the same method createTextBinaryHash. The first comment (line 327-329) is incomplete and should be removed.
| * Creates hash for Cell payload according to TON Connect specification. | |
| */ | |
| /** |
| const innerMessage = { | ||
| workchain: message.address.workChain, | ||
| address: message.address.hash, | ||
| domain: message.domain, | ||
| payload: message.payload, | ||
| timestamp: message.timestamp | ||
| } | ||
|
|
There was a problem hiding this comment.
Unused variable innerMessage is created but never used. The variable is constructed on lines 265-271 but serves no purpose. Consider removing it to clean up the code.
| const innerMessage = { | |
| workchain: message.address.workChain, | |
| address: message.address.hash, | |
| domain: message.domain, | |
| payload: message.payload, | |
| timestamp: message.timestamp | |
| } |
| const walletContract = client.open(this.wallet) | ||
| const seqno = await retry(() => walletContract.getSeqno()) | ||
|
|
||
|
|
There was a problem hiding this comment.
[nitpick] Extra blank line. There's an unnecessary blank line on line 222 that should be removed to maintain consistent code formatting.
| import StacksLib from '@/lib/StacksLib' | ||
| import { TON_CHAINS, TON_SIGNING_METHODS } from '@/data/TonData' | ||
| import { tonAddresses } from '@/utils/TonWalletUtil' | ||
| import { getWallet, tonAddresses, tonWallets } from '@/utils/TonWalletUtil' |
There was a problem hiding this comment.
Unused import tonWallets.
| import { getWallet, tonAddresses, tonWallets } from '@/utils/TonWalletUtil' | |
| import { getWallet, tonAddresses } from '@/utils/TonWalletUtil' |
Supersedes #943