Details

Reviewers

kamil
tomek
ginsu
ashoat

Commits

rCOMM41beef91d650: Change SIWE panel props to enable passing complete SIWE message

Summary

This differential changes SIWE panel props so that it is possible to pass complete message to sign.

Test Plan

Check that SIWE features are not broken.

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

marcin created this revision.May 14 2024, 8:46 AM

Herald added a subscriber: ashoat. · View Herald TranscriptMay 14 2024, 8:46 AM

marcin added a parent revision: D12033: Introduce new header to siwe endpoint.May 14 2024, 8:47 AM

marcin added inline comments.May 14 2024, 8:49 AM

lib/types/siwe-types.js
131 ↗	(On Diff #40165)	I am not happy with this but introduction of this field was landed today so I was totally unprepared for this. I will be thinking of better ways to handle this but for now I didn't want to further delay already delayed diffs.

marcin added a child revision: D12035: Modify Rust restore code to avoid fetching latest backup id twice.May 14 2024, 8:57 AM

Harbormaster completed remote builds in B28900: Diff 40165.May 14 2024, 9:03 AM

marcin requested review of this revision.May 14 2024, 9:03 AM

ashoat requested changes to this revision.May 14 2024, 10:09 AM

ashoat retitled this revision from Change SIWE panl props to enable passing complete SIWE message to Change SIWE panel props to enable passing complete SIWE message.

ashoat added inline comments.

native/account/registration/connect-ethereum.react.js
199 ↗	(On Diff #40165)	Shouldn't we memoize all objects like this?
native/account/registration/siwe-backup-message-creation.react.js
60–62 ↗	(On Diff #40165)	Same here
native/account/siwe-panel.react.js
232 ↗	(On Diff #40165)	As described in the prior diff, I think this approach is very unsafe

This revision now requires changes to proceed.May 14 2024, 10:11 AM

Address comments about memoization.
Modify SIWE panel to possibly take nonce, statement and issuedAt from external source and skip nonce generation in such a case. This is necessary for SIWE restore

Harbormaster completed remote builds in B29008: Diff 40317.May 17 2024, 12:54 AM

Minor comments, but there's a lot of them. Would like to review once more before you land

native/account/fullscreen-siwe-panel.react.js
187 ↗	(On Diff #40317)	This can be extracted outside of the component
native/account/registration/connect-ethereum.react.js
193 ↗	(On Diff #40317)	Same here
216 ↗	(On Diff #40317)	capitalize Ethereum
native/account/registration/connect-farcaster.react.js
68 ↗	(On Diff #40317)	Same here
native/account/registration/siwe-backup-message-creation.react.js
53–55 ↗	(On Diff #40317)	Extract outside component
native/account/siwe-panel.react.js
50–54 ↗	(On Diff #40317)	It seems like this would be more accurate. The `nonceType` field isn't strictly necessary, but may improve readability
81–88 ↗	(On Diff #40317)