fix: prevent to print an error message #293
Conversation
Signed-off-by: Wenhao Ho <[email protected]>
|
I wasn't aware of that issue that's good to know. Looking through it and the discussion it was moved to it looks like recreating the provider works too and doesn't force a page reload which would be preferred if it works, do you know if it does? |
Signed-off-by: Wenhao Ho <[email protected]>
|
@Keyrxng I've revised the code by re-creating means, do you mind helping review and test if it fits our an expectation? |
Thanks but since the issue you are aiming to resolve requires another issue that has not been finalized yet it's a little difficult for me to test the problem as it does not exist on Please bare with me and I'll try to push through other PRs today. Please keep in mind that issues that require other issues to be resolved first sometimes cannot be merged until the underlying issue is merged. |
|
| @@ -10,7 +10,8 @@ export async function verifyCurrentNetwork(desiredNetworkId: number) { | |||
| return; | |||
| } | |||
|
|
|||
| const web3provider = new ethers.providers.Web3Provider(window.ethereum); | |||
| let web3provider = new ethers.providers.Web3Provider(window.ethereum); | |||
| web3provider = new ethers.providers.Web3Provider(web3provider.provider); | |||
There was a problem hiding this comment.
The flow without a page reload would be like this:
- page renders and we call this function.
- Assume no network change is required, nothing happens.
- Assume network change is required, the change happens after this in another function.
- We do not update our global signer and this function would only be called again if they clicked claim again, which isn't possible with the current setup as it'll have a loader. Plus these objects are in-memory and not used for txs or anything.
- The user claim event proceeds and fails against the old provider.
I think a better approach might be for this is to be placed inside connect-wallet() which is the place where the signer comes from which is what we are interested in. connect-wallet is called prior to any claim which makes taking the newly updated web3Provider/signer easier.
P.S: Most, if not all PRs should be accompanied by some kind of QA whether that's a screenshot or small video just to prove the logic works as expected. Here, we'd expect to see a couple of network changes and a claim tx going through, seeing the browser console while these happen is also beneficial and appreciated.
There was a problem hiding this comment.
@Keyrxng Did you mean not only verify-current-network.ts but also apply it to connect-wallet.ts?
|
By the way, I don't know why I can't exactly reproduce the
What do you think? |
I'm not 100% sure. I referenced the PR which introduced the bug and that PR has not yet been merged into I wasn't sure which is why I said it would require further debugging in the task spec, which meant in relation to the logic added in the referenced PR as it contained a large refactoring of the core logic. I think your time would be better served resolving other tasks until this task is resolved, the PR was merged and had errors so it was reverted and it has not been fixed by the contributor yet. Sorry about that @whck6. |
|
Closing this because original issue was closed as "not planned" |
Signed-off-by: Wenhao Ho [email protected]
Resolves #285