Add testnet faucet #717
Add testnet faucet #717
Conversation
|
Ready for review? |
There was a problem hiding this comment.
This is really cool feature!!
When I clicked "Receive testnet tokens", it took a couple of seconds without any visual feedback until a success snackbar popped up. I think we can improve this with a loading state, or at least a snackbar that says "tokens requested" or sth.
Secondly, after the success message, I don't see any transactions in my transaction history. I tried refreshing and there's still nothing. Then after refreshing 2 minutes later, I could see the tokens. I did not have enough time to read the success message, maybe we can leave it open a bit longer. Moreover, since we are expecting an incoming transaction, we should refresh the data automatically, same way we refresh them with an interval when sending a tx. WDYT?
Lastly, we could improve the error handling (in the future, no need for now) when the user reaches the quota (not sure how many minutes it is). Currently it shows "Encountered error, please try again in a few minutes", but it could say "you reached the limit, try again in X minutes" or sth like that.
|
Yes, you saw #718 😆 I'm not happy with the current flow either, it's hard to really see that something happened. That's why I wasn't really sure if this PR was actually ready, but it's always good to get a first review. A button spinner + fetching incoming transactions would help a lot.
In the meantime, this could help 💯 |
|
I've improved the flow by adding a "pending" state indicator. Regarding the continuous loading of the incoming TX, I think that I'd be in favour of implementing alephium/alephium-frontend#74 directly (thus replacing our current "fake" pending TXs system altogether). You're the master of our redux state, so WDYT? :) |
Note: there was a newline at the end of the testnet call response. This made the regex and my brain useless for a little while.
And misc fixes
|
So, it took me some time to get in the code, and this work will eventually be replaced by fetching the mempool data directly, but it's now showing incoming faucet TXs using your system. |
| @@ -87,6 +89,11 @@ const DevToolsSettingsSection = () => { | |||
| setSelectedAddress(undefined) | |||
| } | |||
|
|
|||
| const handleFaucetCall = () => { | |||
| defaultAddress && dispatch(receiveTestnetTokens(defaultAddress?.hash)) | |||
| posthog?.capture('Testnet faucet call') | |||
There was a problem hiding this comment.
nit: I would rename this to Requested testnet tokens to match better the namings of the rest of our analytics/
| const fromAddress = state.entities[pendingTransaction.fromAddress] as Address | ||
| const toAddress = state.entities[pendingTransaction.toAddress] as Address | ||
|
|
||
| ;(fromAddress || toAddress).transactions.push(pendingTransaction.hash) |
There was a problem hiding this comment.
Also, if fromAddress is falsy and toAddress isn't, then toAddresss.transactions.push(pendingTransaction.hash) gets called twice, right?
There was a problem hiding this comment.
I think this should ensure that toAddresss.transactions.push(pendingTransaction.hash) gets called only once and also avoids the weird syntax.
if (fromAddress) fromAddress.transactions.push(pendingTransaction.hash)
if (toAddress && toAddress !== fromAddress) toAddress.transactions.push(pendingTransaction.hash)|
|
||
| const settingsSlice = createSlice({ | ||
| name: 'settings', | ||
| initialState, | ||
| reducers: {}, | ||
| extraReducers(builder) { | ||
| builder | ||
| .addCase(localStorageDataMigrated, () => SettingsStorage.load('general') as GeneralSettings) | ||
| .addCase(localStorageDataMigrated, () => SettingsStorage.load('general') as SettingsState) |
There was a problem hiding this comment.
Hm, why this change? The type of the data loaded is GeneralSettings. faucetCallPending is not stored there.
There was a problem hiding this comment.
Because otherwise the compiler will complain, as addCase gets the expected type from initialState.
But this will be solved if I move faucetCallPending to the globalSlice as you proposed below.
Fixes #690 and #718