-
Notifications
You must be signed in to change notification settings - Fork 374
CIP-0146? | Multi-signatures wallet registration and discovery #971
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
CIP-0146? | Multi-signatures wallet registration and discovery #971
Conversation
…cussion link" This reverts commit daae1b8. Added cardano-foundation#971 to the discussion
|
Thanks @Crypto2099 for raising the potential issue at today's CIP editors meeting. To quickly describe the issue mentioned, we (Eternl) are adding multi-sig (native script) support to our upcoming v2 release. With this, we created this specification (harmonization between wallets/dapps) for how you can pre-announce this native script using the aux data field in tx together with metadata. This was the intention of the field in CDDL as described by CIP-1854. Each participant in the multi-sig somehow needs to know about the script, either through public or offline sharing. The discovery process described in CIP draft looks for TXs containing metadata with label 1854 and native scripts that contain the user's multi-sig key cred. The problem as mentioned by Adam is that some malicious user could look for these registration transactions and post their own, mimicking the real one but with a modified script that would allow them to drain the native script without the other participant's involvement. When the participant tries to discover multi-sig wallets that include his multi-sig key cred, they would find both the invalid (scam) wallet in addition to the real one, and might add and fund the wrong one. Is it a problem in real life 🤷♂️ ... but it's sort of similar to the scam tokens we have seen sent around. There isn't much you can do about those except be vigilant and educate yourself that scammers exist and what methods they use. The only solution I can currently think of is that a multi-sig key (cred) can only be used/registered once. All subsequent registrations would need to use the next key index. Thus a scammer could not replay a posted registration re-using an existing key cred. It complicates handling and syncing but currently the only thing I can think of to mitigate this issue. The other solution would be to acknowledge that it's a potential issue we need to educate users about. Feel free to add other potential solutions to the problem. |
rphair
left a comment
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.
@Scitz0 we agreed at today's CIP meeting to assign this a CIP number regardless of how the issue mentioned in #971 (comment) may progress.
Please rename the proposal directory to CIP-0146 and update the proposal link in the first comment above. 🎉
|
Updates pushed according to the above-proposed solution for mitigation against attack vector on public multisig wallet registration. It also contains changes to metadata format and various tweaks to improve the standard. |
|
Brief comment on Registration could you change to off Chain sharing instead of offline as we expect most pre- registrations to be shared online but off Chain, rather than offline. Best Regards |
|
Also what do you think about using CIP-0083 (encrypted message metadata) for a pre-registration tx to each potential wallet member. Downside only works if member wallet addresses are known pre-registration. To much chain bloat? Or is this out of scope for CIP-0146? |
It sounds like this could be one solution (assuming those addresses are known in advance)... @Scitz0 can you confirm or refute? |
|
Clicked the wrong button 😆 |
|
How about adding another scope to CIP88? Like we did in the CIP88v2 for Pool-Daily-Key Registration? |
Encrypted metadata wouldn't solve issue with replay of multisig wallet registration. The only solution I can see is by rotating keys as currently described in CIP. A MultiSig wallet registration contains two parts. The native scripts that define the MultiSig in auxiliary_scripts, alternatively in If you feel encrypting metadata as a way of hiding 1854 metadata is worth looking into, I could add this as an optional feature. In this case, I would reference CIP-83 and not re-define it. So if |
Though I have read through CIP-88 (once) I'm not that familiar with how it works. Could you please in more detail go over what parts of this CIP you think CIP-88 could solve in a better way? Note that it's important to be able to search for native scripts that include the user's 1854-derived pub key to find all MultiSig wallet registrations that it belongs to. So these must be available on-chain to search for to link to the native script. It's also not possible for participants of the MultiSig to all witness this registration as that would defeat the purpose of the registration/discovery flow and people could just off-chain share the registration. Because of this I currently don't see how CIP-88 in this case can solve it better but as I just explained, I haven't looked deep into CIP88 so I might just not see it. |
Updated according to your comment wording for offline to off-chain 👍 |
|
I will try to join the CIP editors meeting on March 4th where we can maybe talk it through further. |
|
@Scitz0 no problem; have put it on the agenda: https://hackmd.io/@cip-editors/107 (cc @Crypto2099 @jinglescode @QSchlegel @gitmachtl) |
…ript to be included for wallet registration to be valid.
rphair
left a comment
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.
thanks @Scitz0 - my understanding was that securing the metadata was the last thing we were waiting for. The rest of this has always looked OK to me (though I don't have practical experience in the field) so I'm ready to approve this after a couple formatting adjustments.
Co-authored-by: Robert Phair <[email protected]>
Co-authored-by: Robert Phair <[email protected]>
Co-authored-by: Ryan <[email protected]>
Co-authored-by: Ryan <[email protected]>
rphair
left a comment
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.
|
@Scitz0 the feedback from the CIP meeting today is that this is ready to merge as soon as #971 (comment) is addressed to everyone's satisfaction (assuming no further challenges are raised). Therefore this is still in |
Additional ImplementorsThe multisig platform from Mesh is also in the process of implementing this CIP. https://github.com/MeshJS/multisig |
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.
thanks @QSchlegel (cc @Scitz0) ... proposing we add this to the Acceptance Criteria as a box to be ticked as soon as CIP-0146 compliance is demonstrated:
|
I made a minor adjustment: if metadata is encrypted, values need to be string arrays, as the length of the content might exceed the 64-character limit. |
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 MeshJS as Path to Active implementor in 0f9b649 as per #971 (comment).
Last Check status for next CIP meeting (https://hackmd.io/@cip-editors/114) was punted from previous meeting when we ran out of time due to a long issue discussion.
Final approval to merge (as previously suggested in #971 (comment)) should be contingent upon @perturbing @Scitz0 conversation #971 (comment) being resolved... I believe it's currently resolved by default since no actual change has been suggested since then. 👀
* feat: CIP-30 extension to capture network magic * Update CIP-XXXX/README.md updated the CIP number in readme Co-authored-by: Robert Phair <[email protected]> * Update CIP-XXXX/README.md added description to the motivation section Co-authored-by: Robert Phair <[email protected]> * Update CIP-XXXX/README.md added description to the rationale section Co-authored-by: Robert Phair <[email protected]> * CIP-0142: changed from CIP-XXXX, readme updated with feedback * CIP-network-magic: removed the assigned CIP number, added discussion link * Revert "CIP-network-magic: removed the assigned CIP number, added discussion link" This reverts commit daae1b8. Added #971 to the discussion * CIP-0142: added #972 to discussions * fix: removed temporary from extension identifier * fix: removed zero padding from the extension naming * fix: adjusted the zero padding inconsistency * fix: acceptance criteria modifications * fix: implemented wallet providers update * fix: need confirmation on Lace * Update CIP-0142/README.md updated Co-authored-by: Robert Phair <[email protected]> * fix: handled duplicated lines --------- Co-authored-by: Robert Phair <[email protected]>
…tion#972) * feat: CIP-30 extension to capture network magic * Update CIP-XXXX/README.md updated the CIP number in readme Co-authored-by: Robert Phair <[email protected]> * Update CIP-XXXX/README.md added description to the motivation section Co-authored-by: Robert Phair <[email protected]> * Update CIP-XXXX/README.md added description to the rationale section Co-authored-by: Robert Phair <[email protected]> * CIP-0142: changed from CIP-XXXX, readme updated with feedback * CIP-network-magic: removed the assigned CIP number, added discussion link * Revert "CIP-network-magic: removed the assigned CIP number, added discussion link" This reverts commit daae1b8. Added cardano-foundation#971 to the discussion * CIP-0142: added cardano-foundation#972 to discussions * fix: removed temporary from extension identifier * fix: removed zero padding from the extension naming * fix: adjusted the zero padding inconsistency * fix: acceptance criteria modifications * fix: implemented wallet providers update * fix: need confirmation on Lace * Update CIP-0142/README.md updated Co-authored-by: Robert Phair <[email protected]> * fix: handled duplicated lines --------- Co-authored-by: Robert Phair <[email protected]>
A new CIP for multi-signatures wallet registration and discovery handling. CIP-1854 describes the meat of multisig (native scripts) but leaves some areas in the peripheral untouched.
Metadatum label 1854 is also added to the CIP-10 registry.
Rendered Proposal