-
Notifications
You must be signed in to change notification settings - Fork 22
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
Address is not EIP-55 #45
Comments
Perhaps this method can be updated? Although you may have a better suggestion. key_service.dart
|
I did provide a fix to my server side, which converts the siweAddress to checksummed, but don't you agree this should be something we send correctly from the client in the first place? Cheers |
Hello @rhamnett, can you provide steps to repro (specific wallet as well)? Coz I tried with our internal Swift wallet (which does support ERC-55) and I have no issues. |
Hello, I am simply verifying the SIWE message in typescript using In my opinion, the message should be transmitted with the correct type of checksummed address in the message, or the checksum is effectively not useful. The message is created in Flutter using your util method:
Server code:
The above code will fail if the address isn't overwritten with error I suppose you could also look to enhance the Many thanks |
Yes, ERC-55 has still to be supported on flutter SDK, however this shouldn't be a reason for wallets to reject/invalidate the signing as you mention in the original comment:
This is what concerns me. 👆 Can you try by replacing this: createMessage: (SIWECreateMessageArgs args) {
// Create SIWE message to be signed.
// You can use our provided formatMessage() method of implement your own
return SIWEUtils.formatMessage(args);
}, With this: createMessage: (SIWECreateMessageArgs args) {
// Create SIWE message to be signed.
// You can use our provided formatMessage() method of implement your own
final authPayload = SessionAuthPayload.fromJson(args.toJson()).copyWith(
chains: [args.chainId],
aud: args.uri,
type: args.type?.t ?? 'eip4361',
);
final account = NamespaceUtils.getAccount(args.address);
final address = EthereumAddress.fromHex(account);
return _appKitModal.appKit!.formatAuthMessage(
iss: 'did:pkh:${args.chainId}:${address.hexEip55}',
cacaoPayload: CacaoRequestPayload.fromSessionAuthPayload(
authPayload,
),
);
}, |
@quetool i think it's simply that the SIWE node implementation checks to ensure the wallet address is EIP-55 ( see https://github.com/spruceid/siwe/blob/940a66ac3dc7443b52d04fcdcebe5a6d889add9b/packages/siwe/lib/client.ts#L428 ) and fails hard. On the flutter side, when the wallet is created from a seedphrase in the example code... the wallet is created with .hex and not .hexEip55. I've added my own fix on both the flutter side and the server side to ensure that a checksummed wallet is generated & checked in the message. I did ask ChatGPT and it says
So I guess this is up to you if you want to adjust your implementation to send a checksummed address. |
We agree! I think there's a miscommunication here LMAO so let's try to strip this convo down:
We are on the same page! Thanks for the feedback! Keep it coming, please! |
OK thanks :) I certainly didn't take anything as if we were disagreeing, I was simply trying to clarify how I'd worked around (I'd changed the way the wallet key is imported to checksum it). Your solution looks to do the job as well, in a different manner. Apologies if any crossed wires and thanks also for the feedback and great communication! |
Let's keep it open as a reminder that we must support ERC-55! 💯 |
Describe the bug
When I connect the appkit modal example to a wallet that uses the latest walletkit, the SIWE fails because the signing message does not use a checksummed address.
Error:
The address in the message should start 0x958C690D6Cb8d4.... note the capital C after 0x958....
The text was updated successfully, but these errors were encountered: