-
Notifications
You must be signed in to change notification settings - Fork 24
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
feat: enable walletconnect #450
base: master
Are you sure you want to change the base?
Conversation
218e722
to
9b85311
Compare
…zeCore fixes the issue
d3f602a
to
98b3ac1
Compare
2240d26
to
3aa74bc
Compare
"assert": "2.0.0", | ||
"buffer": "4.9.2", | ||
"console-browserify": "1.2.0", | ||
"crypto-js": "4.1.1", | ||
"deprecated-react-native-prop-types": "2.3.0", | ||
"events": "1.1.1", | ||
"ethers": "5.7.2", | ||
"events": "3.3.0", |
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 had to update this to the latest version as the old version didn't have events.off
...
I tested most of the features and they worked, but there is no changelog in the repo.
The idea of the lib is to match the node.js implementation, so I'll check each method before merging this PR, but until then, this is open to review
if (typeof btoa === 'undefined') { | ||
global.btoa = function (str) { | ||
return Buffer.from(str, 'binary').toString('base64'); | ||
}; | ||
} | ||
|
||
if (typeof atob === 'undefined') { | ||
global.atob = function (b64Encoded) { | ||
return Buffer.from(b64Encoded, 'base64').toString('binary'); | ||
}; | ||
} |
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.
shims needed by wallet-connect
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.
suggestion: Add this information to the code itself. Either through simple comments or function docstrings.
This way it will be easier to identify when the shims are no longer needed in the future.
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.
Added a simple comment indicating that those shims are needed by wallet-connect
@@ -45,6 +45,7 @@ | |||
* loaded. | |||
*/ | |||
|
|||
import '@walletconnect/react-native-compat'; |
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.
This adds a set of shims, which can be read here:
3aa74bc
to
44e1876
Compare
// Dynamically import walletconnect modules so that it's not loaded in case | ||
// wallet connect is not enabled. | ||
const { Core } = yield call(() => import('@walletconnect/core')); | ||
const { Web3Wallet } = yield call(() => import('@walletconnect/web3wallet')); |
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.
🌟 Nice solution!
44e1876
to
9701573
Compare
4c95a93
to
e9a0644
Compare
Acceptance Criteria
Security Checklist