feat: Use latest toolkit function for useSendTransaction hook #31
Conversation
📝 WalkthroughWalkthroughThe pull request introduces updates to both dependency management and transaction handling within the project. Notably, the Changes
Sequence DiagramsequenceDiagram
participant Client
participant Transaction Hook
participant Blockchain
Client->>Transaction Hook: Initiate Transaction
Transaction Hook->>Blockchain: zetachainWithdraw (BTC)
Transaction Hook->>Blockchain: zetachainWithdraw (ZRC20)
Transaction Hook->>Blockchain: evmDeposit (Native)
Transaction Hook->>Blockchain: evmDeposit (ERC20)
Transaction Hook->>Blockchain: evmDepositAndCall
The sequence diagram illustrates the updated transaction flow, showcasing the new method calls for different transaction types across various blockchain interactions. 📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Have tested: DepositAndSwap
DepositDeposit tBNB(bsc_testnet) |
|
@fadeev pls review |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (1)
src/components/Swap/hooks/useSendTransaction.ts (1)
Line range hint
185-483: Standardize gas price values across transactionsThere is inconsistency in the
gasPricevalues specified in different functions (10000000000and50000000000). To ensure predictable transaction costs and avoid potential overpayment, consider standardizing thegasPriceor making it configurable based on network conditions.Example adjustment:
const TX_OPTIONS_STANDARD = { gasLimit: 7_000_000, - gasPrice: ethers.BigNumber.from("10_000_000_000"), + gasPrice: ethers.BigNumber.from("50_000_000_000"), // Adjusted for consistency };
🧹 Nitpick comments (2)
src/components/Swap/hooks/useSendTransaction.ts (2)
299-312: Utilize constants for transaction options indepositNativeThe
depositNativefunction redefinesrevertOptionsandtxOptions. Consistently applying the extracted constants will reduce redundancy.Update the function accordingly:
const tx = await client.evmDeposit({ amount: sourceAmount, erc20: "", receiver: addressSelected, - revertOptions: { - callOnRevert: false, - onRevertGasLimit: 7000000, - revertAddress: "0x0000000000000000000000000000000000000000", - revertMessage: "0x", - }, + revertOptions: REVERT_OPTIONS, - txOptions: { - gasLimit: 7000000, - gasPrice: ethers.BigNumber.from("50000000000"), - }, + txOptions: { + ...TX_OPTIONS_STANDARD, + gasPrice: ethers.BigNumber.from("50_000_000_000"), + }, });
465-483: Consolidate transaction options incrossChainSwapHandleThe
crossChainSwapHandlefunction definestxOptionsandrevertOptionsinline. For consistency and maintainability, use the predefined constants.Adjust the function to incorporate the constants:
let tx; tx = await client.evmDepositAndCall({ amount: sourceAmount, erc20: sourceTokenSelected.contract, receiver: omnichainSwapContractAddress, - revertOptions: { - callOnRevert: false, - onRevertGasLimit: 7000000, - revertAddress: "0x0000000000000000000000000000000000000000", - revertMessage: "0x", - }, + revertOptions: REVERT_OPTIONS, - txOptions: { - gasLimit: 7000000, - gasPrice: ethers.BigNumber.from("10000000000"), - }, + txOptions: TX_OPTIONS_STANDARD, types: ["address", "bytes", "bool"], values: [zrc20, recipient, withdraw.toString()], });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (2)
package.json(1 hunks)src/components/Swap/hooks/useSendTransaction.ts(5 hunks)
🔇 Additional comments (1)
package.json (1)
37-37: Verify compatibility of the updated@zetachain/toolkitdependencyThe
@zetachain/toolkitversion has been updated to"13.0.0-rc10", a pre-release candidate. Ensure that this version is stable for production use and compatible with the existing codebase. Pre-release versions may contain breaking changes or unresolved issues.Consider running comprehensive tests to confirm that the updated toolkit functions as expected with your application.
| const tx = await client.zetachainWithdraw({ | ||
| amount: sourceAmount, | ||
| recipient: addressSelected, | ||
| zrc20: sourceTokenSelected.contract, | ||
| receiver: bitcoinAddress, | ||
| revertOptions: { | ||
| callOnRevert: false, | ||
| onRevertGasLimit: 7000000, | ||
| revertAddress: "0x0000000000000000000000000000000000000000", | ||
| revertMessage: "0x", | ||
| }, | ||
| txOptions: { | ||
| gasLimit: 7000000, | ||
| gasPrice: ethers.BigNumber.from("10000000000"), | ||
| }, |
There was a problem hiding this comment.
Ensure bitcoinAddress is validated before usage
In the withdrawBTC function, bitcoinAddress is used as the receiver parameter without prior validation. If bitcoinAddress is undefined, it could lead to runtime errors or unintended behavior during transaction execution.
Apply this diff to add a validation check for bitcoinAddress:
m.withdrawBTC = async () => {
if (!sourceTokenSelected || !destinationTokenSelected) {
return;
}
+ if (!bitcoinAddress) {
+ throw new Error("Bitcoin address is undefined.");
+ }
const from = sourceTokenSelected.chain_name;
const to = destinationTokenSelected.chain_name;
const token = sourceTokenSelected.symbol;Committable suggestion skipped: line range outside the PR's diff.
| const tx = await client.zetachainWithdraw({ | ||
| amount: sourceAmount, | ||
| zrc20, | ||
| recipient: addressSelected, | ||
| receiver: addressSelected, | ||
| revertOptions: { | ||
| callOnRevert: false, | ||
| onRevertGasLimit: 7000000, | ||
| revertAddress: "0x0000000000000000000000000000000000000000", | ||
| revertMessage: "0x", | ||
| }, | ||
| txOptions: { | ||
| gasLimit: 7000000, | ||
| gasPrice: ethers.BigNumber.from("10000000000"), | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Apply standardized constants for revertOptions and txOptions
In the withdrawZRC20 function, the same revertOptions and txOptions are redefined. Utilize the previously extracted constants to enhance consistency and maintainability.
Modify the function to use the constants:
const tx = await client.zetachainWithdraw({
amount: sourceAmount,
zrc20,
receiver: addressSelected,
- revertOptions: {
- callOnRevert: false,
- onRevertGasLimit: 7000000,
- revertAddress: "0x0000000000000000000000000000000000000000",
- revertMessage: "0x",
- },
+ revertOptions: REVERT_OPTIONS,
- txOptions: {
- gasLimit: 7000000,
- gasPrice: ethers.BigNumber.from("10000000000"),
- },
+ txOptions: TX_OPTIONS_STANDARD,
});Committable suggestion skipped: line range outside the PR's diff.
| const from = sourceTokenSelected.chain_name; | ||
| const to = destinationTokenSelected.chain_name; | ||
| const token = sourceTokenSelected.symbol; | ||
| const tx = await client.evmDeposit({ | ||
| amount: sourceAmount, | ||
| erc20: sourceTokenSelected.contract, | ||
| receiver: addressSelected, | ||
| revertOptions: { | ||
| callOnRevert: false, | ||
| onRevertGasLimit: 7000000, | ||
| revertAddress: "0x0000000000000000000000000000000000000000", | ||
| revertMessage: "0x", | ||
| }, | ||
| txOptions: { | ||
| gasLimit: 7000000, | ||
| gasPrice: ethers.BigNumber.from("50000000000"), | ||
| }, | ||
| }); | ||
| if (track) { | ||
| track({ | ||
| hash: tx.hash, | ||
| desc: `Sent ${sourceAmount} ${token} from ${from} to ${to}`, | ||
| }); | ||
| } | ||
| console.log(tx.hash); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor depositERC20 to use standardized options
In the depositERC20 function, revertOptions and txOptions are again duplicated. Apply the constants to maintain uniformity across transaction functions.
Revise the function as follows:
const tx = await client.evmDeposit({
amount: sourceAmount,
erc20: sourceTokenSelected.contract,
receiver: addressSelected,
- revertOptions: {
- callOnRevert: false,
- onRevertGasLimit: 7000000,
- revertAddress: "0x0000000000000000000000000000000000000000",
- revertMessage: "0x",
- },
+ revertOptions: REVERT_OPTIONS,
- txOptions: {
- gasLimit: 7000000,
- gasPrice: ethers.BigNumber.from("50000000000"),
- },
+ txOptions: {
+ ...TX_OPTIONS_STANDARD,
+ gasPrice: ethers.BigNumber.from("50_000_000_000"),
+ },
});Committable suggestion skipped: line range outside the PR's diff.
| revertOptions: { | ||
| callOnRevert: false, | ||
| onRevertGasLimit: 7000000, | ||
| revertAddress: "0x0000000000000000000000000000000000000000", | ||
| revertMessage: "0x", | ||
| }, | ||
| txOptions: { | ||
| gasLimit: 7000000, | ||
| gasPrice: ethers.BigNumber.from("10000000000"), | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor revertOptions and txOptions into reusable constants
The revertOptions and txOptions objects are duplicated across multiple functions, which hampers maintainability and increases the risk of inconsistencies. Extracting these into constants will promote code reuse and simplify future updates.
Define the constants at the appropriate scope:
+const REVERT_OPTIONS = {
+ callOnRevert: false,
+ onRevertGasLimit: 7_000_000,
+ revertAddress: "0x0000000000000000000000000000000000000000",
+ revertMessage: "0x",
+};
+const TX_OPTIONS_STANDARD = {
+ gasLimit: 7_000_000,
+ gasPrice: ethers.BigNumber.from("10_000_000_000"),
+};
m.withdrawBTC = async () => {
// Existing code...
const tx = await client.zetachainWithdraw({
amount: sourceAmount,
zrc20: sourceTokenSelected.contract,
receiver: bitcoinAddress,
- revertOptions: {
- callOnRevert: false,
- onRevertGasLimit: 7000000,
- revertAddress: "0x0000000000000000000000000000000000000000",
- revertMessage: "0x",
- },
+ revertOptions: REVERT_OPTIONS,
- txOptions: {
- gasLimit: 7000000,
- gasPrice: ethers.BigNumber.from("10000000000"),
- },
+ txOptions: TX_OPTIONS_STANDARD,
});Committable suggestion skipped: line range outside the PR's diff.
|
Please, update to: So that it uses dRPC from the networks package, so it's easier to test. |
|
I was able to broadcast a tx: https://sepolia.etherscan.io/tx/0xe13d40bade8705a0adaf0a323b05407e2fd77083b6f020c61120d2e484c6b770 But it's been pending for a while now. Maybe something to do with gas? |
It seems to be the problem of Gas. Sepolia network maintains very high Gas, which makes the test very difficult. Now it shows that more than 1ETH Gas is needed to complete the transaction within 60s, and I don't have enough Gas for the test at present. |
|
One last thing, please, run: Without this, the app doesn't build. Other than that, good to go. |
Done |
2 participants
issue: #30
Summary by CodeRabbit
Dependency Update
@zetachain/toolkitto version13.0.0-rc11@zetachain/protocol-contracts-solanawith version2.0.0-rc1Transaction Handling Improvements