Add useNameRenew Hook #2088
Add useNameRenew Hook #2088
Conversation
|
@cryptobeijing is attempting to deploy a commit to the Coinbase Team on Vercel. A member of the Team first needs to authorize it. |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
🟡 Heimdall Review Status
|
|
|
@cryptobeijing Thanks so much! 🙌 We'll do our best to review this soon 🙏 |
|
JFrankfurt
requested review from
kirkas,
JFrankfurt,
stevieraykatz and
brendan-defi
|
@cryptobeijing thanks very much for your submission. really cool to see such a significant PR 👏 . With that said, while there's a lot here to like, this isn't the UX that we would implement for this experience. What I'd recommend is to focus on a hook specifically for managing renewal transactions. This hook could then be repurposed throughout the codebase wherever we want to initiate renewals. Let me know if you think you're up for that. |
| @@ -33,6 +33,11 @@ export type UseWriteContractWithReceiptProps = { | |||
| eventName: string; | |||
| }; | |||
|
|
|||
| // Extend ContractFunctionParameters to include value property | |||
| export type PayableContractFunctionParameters = ContractFunctionParameters & { | |||
| value?: bigint; | |||
There was a problem hiding this comment.
seems wrong to have this be optional. in the case where we want it, it's required. in the case where we don't want it, it's irrelevant.
let's try to figure out a way to create a discriminated union, based on whether the specified ABI method is payable
There was a problem hiding this comment.
Hi @brendan-defi Thanks for your comments! I will follow your suggestions and focus on a hook. And I will test it with UI locally and only commit the hook.
| await initiateTransaction({ | ||
| abi: REGISTER_CONTRACT_ABI, | ||
| address: REGISTER_CONTRACT_ADDRESSES[connectedChain.id], | ||
| functionName: 'renew', | ||
| args: [selectedName, secondsInYears(years)], | ||
| value: lastQueryPrice, | ||
| }); |
There was a problem hiding this comment.
this is great, let's extract into a reusable hook
There was a problem hiding this comment.
Got it! Thanks for your detailed comments!
cryptobeijing
force-pushed
the
cbj/renewbasename
branch
from
30bcb10 to
1a63f4c
Compare
|
@brendan-defi Hi, I have modified the commit based on your comments. pls review the commit again. Thanks! |
Add useNameRenew Hook
The
useNameRenewhook provides functionality for renewing Basenames. It handles the entire renewal process including price calculation, network switching, and transaction execution.Features
Implementation Details
The hook:
useRentPriceto calculate renewal costsHow has it been tested?
The changes have been tested through:
Unit Tests:
Integration Tests:
Manual Testing:
Contract Verification:
The functionality has been tested with both MetaMask and Coinbase Wallet to ensure broad compatibility.
Have you tested the following pages?
BaseWeb