-
Notifications
You must be signed in to change notification settings - Fork 0
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.
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
Upgrade to useDApps/core 1.2.11 #23
Conversation
Because chainId returns undefined while switching the provider on useEthers activate/deactivate.
dc07f0d
to
26b281d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’ve checked your fix. There were 2 points where I think await is missing, although they are not part of this fix.
One is commented and the other is below.
https://github.com/CNNouns/cnnouns-monorepo/pull/23/files#diff-aefeb8faf6b9330978ae529f732f9d79575267e925c501f429f462be0b76bab8R137
@@ -59,8 +59,7 @@ | |||
"redux": "^4.1.0", | |||
"redux-devtools-extension": "^2.13.9", | |||
"remark-breaks": "^3.0.1", | |||
"web-vitals": "^1.0.1", | |||
"react-transition-group": "^4.4.5" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📝 Duplicate L58
nounsAuctionHouseContract, | ||
undefined, | ||
library && 'getSigner' in library ? library.getSigner() : undefined, | ||
); | ||
const gasLimit = await contract.estimateGas.settleCurrentAndCreateNewAuction(...args); | ||
settleAuction(...args, { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
await is missing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wagyu298 I have reviewed your fix. LGTM!
#25 merged, test passed. |
@ms76 Thank you for your code reviews! |
Reduce
eth_chainId
API call by using StaticJsonRpcProvider.useDApps/core 1.0.8 or later uses StaticJsonRpcProvider for readOnlyUrls.
See ethers-io/ethers.js#901 for more details.