useGasEstimate hook#569
Conversation
|
Warning Rate limit exceeded@Agilulfo1820 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 10 minutes and 22 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughRenames the gas estimation function from Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/vechain-kit/src/hooks/thor/transactions/useGasEstimate.ts (1)
4-12: Critical: Rename to remove "use" prefix - this is not a React hook.The function is named
useGasEstimatebut it's actually a plain async utility function, not a React hook. In React, the "use" prefix is reserved for hooks that use other React hooks internally and follow the Rules of Hooks. This function:
- Returns a
Promise<number>, not hook state- Uses no React hooks internally
- Is meant to be called inside callbacks/async functions
This naming violation:
- Triggers false-positive static analysis errors (see Biome warning on line 156 in useSendTransaction.ts)
- Confuses developers about proper usage patterns
- Violates React naming conventions
🔎 Proposed fix: Rename to follow proper conventions
-export const useGasEstimate = async ( +export const estimateGas = async ( thor: ThorClient, clauses: TransactionClause[], caller: string, options?: { revision?: Revision; gasPadding?: number; }, ) => {You'll also need to update the import in
useSendTransaction.tsand the export inindex.ts.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/vechain-kit/src/hooks/thor/transactions/index.tspackages/vechain-kit/src/hooks/thor/transactions/useGasEstimate.tspackages/vechain-kit/src/hooks/thor/transactions/useSendTransaction.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/migration-guide-to-v2.mdc)
**/*.{ts,tsx}: In VeChain Kit Version 2, useuseThorinstead ofuseConnexfor contract interactions
For single contract read operations, use theuseCallClausehook with the pattern: import dependencies, define ABI and method as const, create query key function usinggetCallClauseQueryKeyWithArgs, and wrap with useCallClause including data transformation in the select option
For multiple parallel contract calls, use theexecuteMultipleClausesCallutility wrapped in auseQueryhook with the pattern: define query key function, useexecuteMultipleClausesCallin queryFn mapping items to clause objects, and transform results
For transaction building and sending, use theuseBuildTransactionhook with a clauseBuilder function that returns an array of clauses with optional comment fields describing the transaction action
Always provide an arguments array for contract calls, even when no parameters are required - use an empty array for parameter-less functions to enable TypeScript type checking
Always conditionally enable queries using theenabledproperty to prevent unnecessary contract calls, checking for all required parameters:enabled: !!requiredParam && !!otherRequiredParam
Use theselectoption in useCallClause or transform data in queryFn to handle data transformation, particularly for converting BigInt values to strings and normalizing contract return data
Maintain consistent query key patterns: usegetCallClauseQueryKeyWithArgsfor contract calls with arguments andgetCallClauseQueryKeyfor calls without arguments to ensure proper caching and invalidation
Use TypeScriptas constassertions for method names andas0x${string}`` assertions for Ethereum addresses to ensure type safety in contract interactions
Files:
packages/vechain-kit/src/hooks/thor/transactions/index.tspackages/vechain-kit/src/hooks/thor/transactions/useSendTransaction.tspackages/vechain-kit/src/hooks/thor/transactions/useGasEstimate.ts
🧠 Learnings (3)
📚 Learning: 2025-12-01T13:01:33.771Z
Learnt from: CR
Repo: vechain/vechain-kit PR: 0
File: .cursor/rules/migration-guide-to-v2.mdc:0-0
Timestamp: 2025-12-01T13:01:33.771Z
Learning: Applies to **/*.{ts,tsx} : In VeChain Kit Version 2, use `useThor` instead of `useConnex` for contract interactions
Applied to files:
packages/vechain-kit/src/hooks/thor/transactions/index.tspackages/vechain-kit/src/hooks/thor/transactions/useSendTransaction.tspackages/vechain-kit/src/hooks/thor/transactions/useGasEstimate.ts
📚 Learning: 2025-12-01T13:01:33.771Z
Learnt from: CR
Repo: vechain/vechain-kit PR: 0
File: .cursor/rules/migration-guide-to-v2.mdc:0-0
Timestamp: 2025-12-01T13:01:33.771Z
Learning: Applies to **/*.{ts,tsx} : For transaction building and sending, use the `useBuildTransaction` hook with a clauseBuilder function that returns an array of clauses with optional comment fields describing the transaction action
Applied to files:
packages/vechain-kit/src/hooks/thor/transactions/useSendTransaction.tspackages/vechain-kit/src/hooks/thor/transactions/useGasEstimate.ts
📚 Learning: 2025-12-01T13:01:33.771Z
Learnt from: CR
Repo: vechain/vechain-kit PR: 0
File: .cursor/rules/migration-guide-to-v2.mdc:0-0
Timestamp: 2025-12-01T13:01:33.771Z
Learning: Applies to **/*.{ts,tsx} : For single contract read operations, use the `useCallClause` hook with the pattern: import dependencies, define ABI and method as const, create query key function using `getCallClauseQueryKeyWithArgs`, and wrap with useCallClause including data transformation in the select option
Applied to files:
packages/vechain-kit/src/hooks/thor/transactions/useGasEstimate.ts
🧬 Code graph analysis (1)
packages/vechain-kit/src/hooks/thor/transactions/useSendTransaction.ts (1)
packages/vechain-kit/src/hooks/thor/transactions/useGasEstimate.ts (1)
useGasEstimate(4-28)
🪛 Biome (2.1.2)
packages/vechain-kit/src/hooks/thor/transactions/useSendTransaction.ts
[error] 156-157: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Lint & Build
- GitHub Check: e2e-test
- GitHub Check: build-size-check
- GitHub Check: deploy
🔇 Additional comments (2)
packages/vechain-kit/src/hooks/thor/transactions/useSendTransaction.ts (2)
156-164: Static analysis error is caused by misleading function name.The Biome linter correctly flags this as calling a hook conditionally, but this is a false positive caused by the naming of
useGasEstimate. The function is not actually a React hook—it's a plain async utility function. Once renamed (see comment on useGasEstimate.ts), this static analysis error will resolve.The actual usage pattern here is correct for an async utility function: proper try-catch error handling and appropriate parameter passing.
195-195: Good addition to dependency array.Adding
requestTransactionto the dependency array is correct since it's used in the callback at line 169. This ensures the callback is recreated whenrequestTransactionchanges.
|
Size Change: +926 B (+0.02%) Total Size: 5.74 MB
ℹ️ View Unchanged
|
renaming to use this for estimating gas, have tried it out for veworld and smart account txs.
Closes #466
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.