Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: prevent to print an error message #293

Closed
wants to merge 2 commits into from
Closed

fix: prevent to print an error message #293

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e5237b5
fix: prevent to print an error message
whck6 Sep 8, 2024
06ec962
fix: avoid using the any network
whck6 Sep 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion static/scripts/rewards/web3/verify-current-network.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Contributor

@Keyrxng Keyrxng Sep 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The flow without a page reload would be like this:

  1. page renders and we call this function.
  2. Assume no network change is required, nothing happens.
  1. Assume network change is required, the change happens after this in another function.
  2. 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.
  3. 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.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Keyrxng Did you mean not only verify-current-network.ts but also apply it to connect-wallet.ts?


const network = await web3provider.getNetwork();
const currentNetworkId = network.chainId;
Expand Down
Loading