Skip to content

Add max secret length#234

Closed
thesimplekid wants to merge 2 commits intomainfrom
thesimplekid-patch-1
Closed

Add max secret length#234
thesimplekid wants to merge 2 commits intomainfrom
thesimplekid-patch-1

Conversation

@thesimplekid
Copy link
Collaborator

@thesimplekid thesimplekid commented Mar 6, 2025

@elnosh
Copy link
Contributor

elnosh commented Mar 20, 2025

cool, I was wondering about what limit to use for secrets. Since this is already in nutshell, I'm assuming it does not cause much trouble for secrets in P2PK and HTLCs?

@thesimplekid
Copy link
Collaborator Author

I'm assuming it does not cause much trouble for secrets in P2PK and HTLCs?

Its a bit of an unknown I think it hasn't caused an issue yet but we don't know how long we may want these scripts to be in the future. Though expanding the limit the the future would be easier then trying to limit it once long secrets are used if they do become a problem.

@elnosh
Copy link
Contributor

elnosh commented Mar 25, 2025

should we add an error code for this?

@robwoodgate
Copy link
Contributor

robwoodgate commented Apr 10, 2025

For P2PK, 512 characters seems to allow 5 (sometimes 4!) pubkeys total, split over data, pubkeys and refund.
eg allows an n-of-4 multisig with one refund key... n-of-2 multisig with 3 refund keys etc.

A limit on secret length is really not ideal because a mint can sign a blinded message and allow user to create a proof that it will later not allow to be redeemed if secret is too long.

00.md Outdated
```

`amount` is the amount of the `Proof`, `secret` is the secret message and is a utf-8 encoded string (the use of a 64 character hex string generated from 32 random bytes is recommended to prevent fingerprinting), `C` is the unblinded signature on `secret` (hex string), `id` is the [keyset id][02] of the mint public keys that signed the token (hex string).
`amount` is the amount of the `Proof`, `secret` is the secret message and is a utf-8 encoded string that **MUST NOT** exceed 512 characters (the use of a 64 character hex string generated from 32 random bytes is recommended to prevent fingerprinting), `C` is the unblinded signature on `secret` (hex string), `id` is the [keyset id][02] of the mint public keys that signed the token (hex string).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternative approach worth considering is a double-standard between vanilla and NUT-10 secrets:

  • VANILLA: let's decide a fixed size for vanilla secrets (64 character hex is good)
  • NUT-10: Can be longer than 64 characters and we decide on a case by case basis:
    • NUT-11: Let's take this example:
[
  "P2PK",
  {
    "nonce": "da62796403af76c80cd6ce9153ed3746",
    "data": "033281c37677ea273eb7183b783067f5244933ef78d8c3f15b1a77cb246099c26e",
    "tags": [
      ["sigflag", "SIG_ALL"],
      ["n_sigs", "2"],
      ["locktime", "1689418329"],
      [
        "refund",
        "033281c37677ea273eb7183b783067f5244933ef78d8c3f15b1a77cb246099c26e"
      ],
      [
        "pubkeys",
        "02698c4e2b5f9534cd0687d87513c759790cf829aa5739184a3e3735471fbda904",
        "023192200a0cfd3867e48eb63b03ff599c7e46c8f4e41146b2d281173ca6c50c54"
      ]
    ]
  }
]

You can see how the variable length field is only ever pubkeys. All the rest is fixed size. Just add a cap size to the number of pubkeys.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed size instead of a max size sounds good!

`amount` is the amount of the `Proof`, `secret` is the secret message and is a utf-8 encoded string (the use of a 64 character hex string generated from 32 random bytes is recommended to prevent fingerprinting), `C` is the unblinded signature on `secret` (hex string), `id` is the [keyset id][02] of the mint public keys that signed the token (hex string).
`amount` is the amount of the `Proof`, `secret` is the secret message and is a utf-8 encoded string that **MUST NOT** exceed 512 characters (the use of a 64 character hex string generated from 32 random bytes is recommended to prevent fingerprinting), `C` is the unblinded signature on `secret` (hex string), `id` is the [keyset id][02] of the mint public keys that signed the token (hex string).

## 0.2 - Protocol
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For NUT-11, refund is variable length, same as pubkeys. From testing, around 4-5 total pubkeys (spread over data, refund and pubkeys) will reach the 512 limit.

Perhaps not a hard character limit (as that requires calculating secret before submitting a swap/melt)... but maybe a total limit on pubkeys over all three fields.

Eg a limit of, say, 5 would allow an n-of-5 multisig with no refund keys, or single lock (data) key with 4 refund keys.

Overall I'd prefer a case-by-case limit as suggested by @lollerfirst over a hard 512 limit to allow flexibility for future NUT-10 spending conditions

Copy link
Contributor

@a1denvalu3 a1denvalu3 Apr 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good idea although data is not variable length. Maybe a hard-cap on the number of pubkeys over refund and pubkeys would do (for NUT-11)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, data isn't variable, but is a public key field for P2PK. I mentioned it just because it is a locking PK field.. eg an n-of-3 multisig would include data key plus 2 keys in pubkeys.

But essentially, for NUT-11, a limit on total keys over refund and pubkeys is where I'd prefer to go.

@thesimplekid
Copy link
Collaborator Author

Closing in favor of #255

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants