fix(unlock-app): checkout frame errors #14912
Conversation
| const buttonText = approved | ||
| ? 'Purchase a key' | ||
| : `Approve ${lock.currencySymbol} spending` |
There was a problem hiding this comment.
| const buttonText = approved | |
| ? 'Purchase a key' | |
| : `Approve ${lock.currencySymbol} spending` | |
| const buttonText = approved | |
| ? 'Mint' | |
| : `Approve ${lock.currencySymbol} spending` |
| @@ -42,6 +73,16 @@ export async function getLockDataFromCheckout(id: string) { | |||
| const res = await web3Service.getLock(lockAddress, network) | |||
| const price = `${res.keyPrice} ${res.currencySymbol}` | |||
|
|
|||
| let tokenAddress | |||
| if (res.currencySymbol !== 'ETH') { | |||
There was a problem hiding this comment.
That's not the right way to check if the lock is priced in the native currency. Thus would fail on Polygon for example. Just check that the lock.tokenAddress is the Zero address.
| const tokens = networks[network].tokens | ||
| const matches = tokens!.filter( | ||
| (token) => token.symbol === res.currencySymbol | ||
| ) | ||
| tokenAddress = matches.find((token) => token.featured) || matches[0] || null | ||
| tokenAddress = tokenAddress.address |
There was a problem hiding this comment.
That's also not correct. just use lock.tokenAddress here.
In the case of an ERC20 lock, you first need to check if there is an existing approval and if that approval is "enough" ( > price). I don't think you can "batch" the 2 tx. The user would first need to approve and 2nd need to send the purchase. |
No, you need to actually perform a check and can't rely on the fact that there is an error, because the purchase could fail for other reasons (sold out.. etc). What do you think? |
|
to get the user address, they are first required to sign a dummy message to prompt the wallet connect. The user's address is only available in a transaction context so this hacky way is the only one I see to get that without an error or an actual transaction. https://www.loom.com/share/722c2376444a4b6393963e43a8398669?sid=a407e3b8-a2d5-483a-a2a9-e5971bc7754d |
You can get it from the frame as long as the user has interracted with it, which is why the "Continue" button is useful :) |
This might be specific to the current version of framesjs or only on the local debugger not sure, but only a button with action 'mint' or 'tx' is able to give a prompt to connect a wallet. the continue button would have a 'post' or 'post_redirect' property which doesn't connect a wallet but only navigates to another page (I just confirmed locally that it doesn't). Maybe in prod it might behave differently but in the docs I don't see any other way to get the address with no tx. https://framesjs.org/guides/transactions#using-the-connected-wallet-address Which kind of action button ( https://framesjs.org/reference/core/Button#button ) do you suggest for 'continue'? I seem to hit a wall on that one 😮💨 |
| const lock = ctx.state.lock! | ||
| const { address: lockAddress, network, priceForUser } = lock | ||
|
|
||
| const calldata = encodeFunctionData({ |
There was a problem hiding this comment.
You don't need to use that! unlock-js has the logic you need to use.
There was a problem hiding this comment.
I didn’t catch that, the encodeFunctionData part or the entire erc20 approve 🤔
There was a problem hiding this comment.
So I think you should first check if the user has already approved enough (and you can compute the amount that needs to be approved based on the ehckout config (especially if it supports renewals)
Sorry my initial comment was unclear. Check unlock-js where there is already logic around how to compute the amount to be approved.
There was a problem hiding this comment.
seen the logic in unlock-js/src/PublicLock/v10/getPurchaseKeysArguments to account for renewals and done it slightly differently.
my approach now checks if a lock is renewable to decide whether to render the approve button, so you can always approve a different amount before the purchase to replace any previous approval
You have the |
| userAddress as string, | ||
| tokenAddress! | ||
| ) | ||
| if (Number(allowance) >= Number(keyPrice)) { |
There was a problem hiding this comment.
That's not enough, the user may need to approve MORE than the keyprice to allow for renewals.
| return price | ||
| } | ||
|
|
||
| const keyPrice = await getKeyPrice() | ||
|
|
||
| const calldata = encodeFunctionData({ |
There was a problem hiding this comment.
Please use unlock'js walletService here as well so this supports any version of the lock contract!
There was a problem hiding this comment.
I ran into errors getting the abi with walletService.lockContractAbiVersion() which seems the closest method in functionality imo.
Would it work for all versions just importing the generic PublicLock without a version, or i'll need to dig into walletservice a bit more 🤔
Description
Issues
Fixes #14888
Refs #
Checklist:
Indeed an erc20 approval is needed first. This handles that by having the first button click as the approval and the second as the actual purchase, lmk if that's confusing or a more elegant way, I don't see a way to 'batch' those 2 together in a single transaction so far 🤔 .
As for checking if a user has a membership before the purchase, this can happen by cancelling a transaction midway, because the user's wallet details are not accessible elsewhere. Doing that would throw an error anyway so I'm assuming a failed purchase (because you already have a key) does the same and it's sort of an 'acceptable' error
Frame.Debugger.mp4
Release Note Draft Snippet