search

stacksgov / sips

Community-submitted Stacks Improvement Proposals (SIPs)
133 stars 80 forks source link

SIP-018 Signed Structured Data #57

Closed MarvinJanssen closed 1 year ago

MarvinJanssen commented 2 years ago

The Signed Structured Data specification proposed in this SIP describes a standard way to deterministically encode and sign Structured Data. Structured Data is data that can be represented in human-readable format and used in applications and smart contracts. The aim of the standard is to produce signatures that are straightforward and inexpensive to verify by smart contracts.

Reference implementation:

Requires:

See also:

jcnelson commented 2 years ago

I took another pass at this, in light of the recent OpenSea phishing attack. While I stand by my belief that discussion of to-consensus-buff should be part of the Stacks 2.1 SIP, I think the rest of this is good. Most of my comments are just asking for minor clarifications.

friedger commented 2 years ago

Just as additional context, Verifiable Credentials are a second type of structured data that wallets should support

radicleart commented 2 years ago

Marvin - a question on making the domain info stronger by eg including domain names and user agent info?

cf. EIP 712 says this..

The way to solve this is by introducing a domain separator, a 256-bit number. This is a value unique to each domain that is ‘mixed in’ the signature. It makes signatures from different domains incompatible. The domain separator is designed to include bits of DApp unique information such as the name of the DApp, the intended validator contract address, the expected DApp domain name, etc. The user and user-agent can use this information to mitigate phishing attacks, where a malicious DApp tries to trick the user into signing a message for another DApp.

aulneau commented 2 years ago

Hi all -- just wanted to let folks know that I've implemented support for interacting with wallets that support this standard within micro-stacks:

Relevant versions: micro-stacks: >= 0.5.0 @micro-stacks/react: >= 0.2.1

Some docs: https://docs.micro-stacks.dev/modules/core/connect/message-signing https://docs.micro-stacks.dev/modules/integrations/react/message-signing

Note: I have also implemented support for recoverable signatures in both RSV mode (like this sip expects) and VRS mode (like legacy stacks functions used)

jcnelson commented 2 years ago

Did another pass. Let's go ahead and advance this to Accepted :tada:

Before we can advance to Recommended, I have two outstanding comments that need to be addressed. Also, @kantai (who is now on the technical CAB) needs to make a pass as well.

MarvinJanssen commented 2 years ago

I added test vectors at the end of the SIP per @janniks in 395ea83 and moved it to Accepted status. Still an open question regarding the prefix. (See comment above.)

MarvinJanssen commented 2 years ago

@radicleart I'm wary adding a domain name to the domain object. I can foresee future applications that might not use a domain name or do not want to be strongly coupled to one. For example, an open DeFi protocol built on top of SIP018 might be used across different applications. The protocol then simply defines a unique name or other identifier in the domain object. That is not to say a future standard based on SIP018 cannot introduce a domain name in the domain object for specific purposes (like app sign in).

jiga commented 2 years ago

I propose adding signature timestamp and signature expiration timestamp fields as part of domain. this may help mitigate potential misuse of signed messages e.g. prevent replay of meta transactions by compromised dapps ideally signature timestamp to be autogenerated while signing messages

MarvinJanssen commented 2 years ago

@jiga I don't think those fields should be part of SIP018 as I don't think they are always desirable. Furthermore, those fields should probably go in the payload and not the domain. (An order tuple may have an expiry field, for example.)

Meta transactions are a valid use case and I would suggest a future meta transaction SIP to be formulated on top of SIP018.

MarvinJanssen commented 2 years ago

SIP document updated with the new "SIP018" prefix in 318bad3!

To everyone that was/is implementing SIP018: this is a BREAKING CHANGE.

The prefix has been changed from a single-byte 0xC0 to six bytes: [0x53, 0x49, 0x50, 0x30, 0x31, 0x38]. (Which spells "SIP018" in ASCII.)

If you are curious why, see this discussion: https://github.com/stacksgov/sips/pull/57#discussion_r776508478

The reference implementation has also been updated to reflect the change:

aulneau commented 2 years ago

micro-stacks@0.5.1 has been updated to support the latest implementation of this SIP, matching the test vectors supplied in https://github.com/stacksgov/sips/commit/318bad31f09fb60b02181f1d1c81b26ba8254408

kantai commented 2 years ago

This looks great to me, just had some superficial comments, and then we can move this to recommended!

MarvinJanssen commented 2 years ago

Implemented suggestions and moved to Recommended.

jcnelson commented 2 years ago

Cool! Do the authors feel that this is ready for activation?

MarvinJanssen commented 2 years ago

Yep. But will give @janniks time to respond to the last open conversation.

jcnelson commented 2 years ago

Wanna ping me when it's ready? @janniks @MarvinJanssen

MarvinJanssen commented 2 years ago

Ready to go @jcnelson

jcnelson commented 2 years ago

Cool, it's now listed as "activating" in the README. Can you set the status to Activation-in-Progress? Thanks!

MarvinJanssen commented 2 years ago

Done!

MarvinJanssen commented 1 year ago

Looking at the activation criteria:

  1. to-consensus-buff or equivalent functionality is added to Stacks 2.1.
  2. At least one popular wallet application implements the standard before or within one year of the launch of Stacks 2.1.

Do you think both have been met @jcnelson? Hiro Wallet as well as the Stacks Ledger applet added support for SIP018 messages and to-consensus-buff has been added to Stacks 2.1. Can we thus consider this SIP activated or do we wait until the launch of Stacks 2.1?

jcnelson commented 1 year ago

Just an update on this -- we will merge this when 2.1 activates on mainnet.