Add connectAndSignIn to CBWProvider #910
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bangtoven
commented
Jun 30, 2023
Comment on lines
+605
to
+607
| * @param {string} params.nonce - A unique string to prevent replay attacks. | ||
| * @param {string} [params.statement] - An optional human-readable ASCII assertion that the user will sign. | ||
| * @param {string[]} [params.resources] - An optional list of information the user wishes to have resolved as part of authentication by the relying party. |
There was a problem hiding this comment.
This reverts commit fec69ba.
bangtoven
commented
Jun 30, 2023
| @@ -596,6 +597,76 @@ export class CoinbaseWalletProvider | |||
| return res.result; | |||
| } | |||
|
|
|||
| /** | |||
bangtoven
commented
Jun 30, 2023
Comment on lines
+605
to
+606
| * IMPORTANT: Signature validation is not performed by this method. Users of this method are advised to perform their own signature validation. | ||
| * Common web3 frontend libraries such as ethers.js and viem provide the `verifyMessage` utility function that can be used for signature validation. |
There was a problem hiding this comment.
doesn't seem like there's an existing method to do this.
we have one named ethereumAddressFromSignedMessage, but it's defined on the relay and makes request via WL...
bangtoven
commented
Jun 30, 2023
Comment on lines
+629
to
+631
| // NOTE: It was intentionally built by following the pattern of the existing eth_requestAccounts method | ||
| // to maintain consistency and avoid introducing a new pattern. | ||
| // We acknowledge the need for a better design, and it is planned to address and improve it in a future refactor. |
vishnumad
approved these changes
Jul 1, 2023
Comment on lines
+625
to
+627
| accounts: AddressString[]; | ||
| message: string; | ||
| signature: string; |
There was a problem hiding this comment.
We may want to type message and signature as HexString.
bangtoven
added a commit
that referenced
this pull request
Feb 29, 2024
* add connectAndSignIn * for testing * implementation. * Revert "for testing" This reverts commit fec69ba.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
How did you test your changes?