Repository navigation
feat(BA-2133): ENSIP-19 updates for registration flow for EOAs - #2594
Conversation
✅ Heimdall Review Status
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
69cbf6c to
3e8c3a6
Compare
| const [reverseSigPayload, setReverseSigPayload] = useState<{ | ||
| coinTypes: readonly bigint[]; | ||
| signatureExpiry: bigint; | ||
| signature: `0x${string}`; | ||
| } | null>(null); |
There was a problem hiding this comment.
This useState feels unnecessary since these values are only computed in registerName and don't depend on the react lifecycle
There was a problem hiding this comment.
I switched this to use a ref, which from what I understand helps persist a value across renders though doesn't trigger a re-render if the value changes. Are you thinking we don't even need a ref because the signature will be used immediately upon its creation?
There was a problem hiding this comment.
Are you thinking we don't even need a ref because the signature will be used immediately upon its creation?
Yes exactly, we don't need these values to persist across renders since the transaction using the signature is initiated in the same function (registerName)
Also, it's generally safer to re-request the signature every time register is clicked because there's currently no cache invalidation implemented. So a flow like this could happen:
- User clicks register
- User signs the message
- User cancels the transaction
- User disconnects and connects a different wallet
- User clicks register
- This time there's no signature request and the transaction re-uses the wrong signature from the last wallet
| @@ -185,11 +253,12 @@ export function useRegisterNameCallback( | |||
| return { | |||
| callback: registerName, | |||
| isPending: registerNameIsLoading || batchCallsIsLoading, | |||
There was a problem hiding this comment.
We should also return a loading state from the signature request here. useSignMessage exposes a status param you can use.
const { signMessageAsync, status } = useSignMessage();
const signMessageIsLoading = status === 'pending'
...
isPending: registerNameIsLoading || batchCallsIsLoading || signMessageIsLoading,| chain: basenameChain, | ||
| account: address, | ||
| } as unknown as WriteContractParameters); |
There was a problem hiding this comment.
Do we need to pass these extra params? also is the type cast needed?
There was a problem hiding this comment.
nope, this is fixing an existing linter error around value : Object literal may only specify known properties, and 'value' does not exist in type 'ContractFunctionParameters'.ts(2353)
But I'm going to revert it to how it was because it doesn't break the build and isn't explicitly necessary for this change
Approved review 3207715161 from arjun-dureja is now dismissed due to new commit. Re-request for approval.
This branch was successfully deployed
4 participants
What changed? Why?
This adds the ability to register basenames (including signing to set the name as primary name in the L2ReverseRegistrar) for EOAs with the new ENSIP-19 compliant contracts.
Notes to reviewers
How has it been tested?
testingpostreviewcomments.mov
SurfaceError.mov
Have you tested the following pages?
BaseWeb