Conversation
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
| const transaction = { | ||
| to: params.token as `0x${string}`, | ||
| data: tokenContract.write.transfer.toString(), // This needs proper encoding | ||
| // Note: This is a simplified version. In practice, you'd need to properly encode the function call |
There was a problem hiding this comment.
The ERC-20 token transfer implementation has an issue with the transaction data encoding. The current approach using tokenContract.write.transfer.toString() won't produce valid transaction data that can be executed on-chain.
For proper ERC-20 transfers, the transaction data needs to be encoded with the function signature and properly formatted parameters. Consider using viem's encodeFunctionData method instead:
import { encodeFunctionData } from 'viem'
// ...
const data = encodeFunctionData({
abi: ERC20_ABI,
functionName: 'transfer',
args: [params.to as `0x${string}`, value]
})
const transaction = {
to: params.token as `0x${string}`,
data
}This will ensure the transaction contains properly encoded calldata that the EVM can interpret correctly when executing the token transfer.
| const transaction = { | |
| to: params.token as `0x${string}`, | |
| data: tokenContract.write.transfer.toString(), // This needs proper encoding | |
| // Note: This is a simplified version. In practice, you'd need to properly encode the function call | |
| const transaction = { | |
| to: params.token as `0x${string}`, | |
| data: encodeFunctionData({ | |
| abi: ERC20_ABI, | |
| functionName: 'transfer', | |
| args: [params.to as `0x${string}`, value] | |
| }) | |
| } |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
| const amount = typeof params.amount === 'string' | ||
| ? Math.floor(parseFloat(params.amount) * 1_000_000_000) | ||
| : typeof params.amount === 'bigint' | ||
| ? Number(params.amount) | ||
| : Math.floor(params.amount * 1_000_000_000); |
There was a problem hiding this comment.
The conversion from bigint to Number in the SUI amount calculation risks precision loss for large values. This is particularly problematic for cryptocurrency transactions where exact amounts are critical. Consider preserving the full precision by using bigint arithmetic throughout:
const amount = typeof params.amount === 'string'
? BigInt(Math.floor(parseFloat(params.amount) * 1_000_000_000))
: typeof params.amount === 'bigint'
? params.amount
: BigInt(Math.floor(params.amount * 1_000_000_000));This approach maintains precision for large transaction amounts while still handling all the input types correctly.
| const amount = typeof params.amount === 'string' | |
| ? Math.floor(parseFloat(params.amount) * 1_000_000_000) | |
| : typeof params.amount === 'bigint' | |
| ? Number(params.amount) | |
| : Math.floor(params.amount * 1_000_000_000); | |
| const amount = typeof params.amount === 'string' | |
| ? BigInt(Math.floor(parseFloat(params.amount) * 1_000_000_000)) | |
| : typeof params.amount === 'bigint' | |
| ? params.amount | |
| : BigInt(Math.floor(params.amount * 1_000_000_000)); |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
There was a problem hiding this comment.
@rafinskipg BigInt is probably the best stable type to use everywhere if possible? Like, just get rid of the fractional part at step 1 and stay in BigInt through everything?
| import { SuiTransactionBuilderImpl } from "./builders/sui"; | ||
|
|
||
| // Export types | ||
| export type * from "./types"; |
There was a problem hiding this comment.
The syntax export type * from "./types"; is not valid TypeScript. TypeScript doesn't support exporting all types with the type keyword in this manner. To properly export all types and interfaces from the types file, change this to export * from "./types"; which will export both types and values.
| export type * from "./types"; | |
| export * from "./types"; |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
| { | ||
| files: ["tsup.config.ts"], | ||
| rules: { | ||
| "import/no-extraneous-dependencies": "off", |
There was a problem hiding this comment.
Do we strictly need to be turning off this rule globally?
| const transaction = { | ||
| to: params.token as `0x${string}`, | ||
| data: tokenContract.write.transfer.toString(), // This needs proper encoding | ||
| // Note: This is a simplified version. In practice, you'd need to properly encode the function call |
| import { SuiTransactionBuilderImpl } from "./builders/sui"; | ||
|
|
||
| // Export types | ||
| export type * from "./types"; |
|
|
||
| // Convert amount to MIST (1 SUI = 1,000,000,000 MIST) | ||
| const amount = typeof params.amount === 'string' | ||
| ? Math.floor(parseFloat(params.amount) * 1_000_000_000) |
There was a problem hiding this comment.
could make these methods generic with some baseTokenToAmount or whatever if you set up a lookup of MIST_PER_SUI / LAMPORTS_PER_SOL etc
| const amount = typeof params.amount === 'string' | ||
| ? Math.floor(parseFloat(params.amount) * 1_000_000_000) | ||
| : typeof params.amount === 'bigint' | ||
| ? Number(params.amount) | ||
| : Math.floor(params.amount * 1_000_000_000); |
There was a problem hiding this comment.
@rafinskipg BigInt is probably the best stable type to use everywhere if possible? Like, just get rid of the fractional part at step 1 and stay in BigInt through everything?
Adds a new transactions package to simplify transaction creation
🔧 Core Features
⚡ Usage Examples