BIP93: correct threshold range #2023
Conversation
jonatack
changed the title
| It includes an encoding format, a BCH error-correcting checksum, and algorithms for share generation and secret recovery. | ||
| Secret data can be split into up to 31 shares. | ||
| A minimum threshold of shares, which can be between 1 and 9, is needed to recover the secret, whereas without sufficient shares, no information about the secret is recoverable. | ||
| A minimum threshold of shares, which can be 0 (for unshared secret) or between 2 and 9 (for shares), is needed to recover the secret, whereas without sufficient shares, no information about the secret is recoverable. |
There was a problem hiding this comment.
Not sure about this change.
The current abstract states "between 1 and 9" for what "is needed to recover the secret".
And the Recovering Master Seed section stipulates: "The first character of the data part indicates the threshold of the share, and it is required to be a non-"0" digit."
So these two excerpts seem to concur? Perhaps it could be clearer.
cc @apoelstra for feedback
There was a problem hiding this comment.
It's correct as it is.
Threshold is a value 1 through 9 and refers to the number of strings needed to recover the seed.
k is the literal first numeric character of the bech32 data so it cannot be "1" even if the threshold is 1. "0" is recommended for unshared secrets (threshold 1) although any numeric value is allowed as it is ignored when share_idx = "s".
Whether it is clear or not is another matter. You'd have to read the body to know these details so I think the abstract is fine. There may be some conflation of "threshold" with "threshold digit" which is called k in the codex book and many reference implementations, which is what lead you to opening this PR.
jonatack
added
Proposed BIP modification
Pending acceptance
jonatack
added
the
PR Author action required
|
Thanks for the review and giving a justified correction! Is this closer to what you suggested? |
bip-0093.mediawiki
Outdated
| * A data part which is in turn subdivided into: | ||
| ** A threshold parameter, which MUST be a single digit between "2" and "9", or the digit "0". | ||
| *** If the threshold parameter is "0" then the share index, defined below, MUST have a value of "s" (or "S"). | ||
| ** A threshold digit (also called ''k'' in the codex book), which MUST be a single digit between "2" and "9", or the digit "0". This digit encodes the threshold (the number of shares required for recovery), where threshold 1 is encoded as "0" for unshared secrets, and thresholds 2-9 are encoded as digits "2"-"9" for shared secrets. |
There was a problem hiding this comment.
I don't know if we need to mention what the digit is called in the Codex32 book.
This digit does not always directly encode the threshold number of strings required for recovery.
Threshold 1 is denoted by share index "s" NOT the first data character being "0". "0" is merely a recommendation, and if used, forces the share index to "s".
| ** A threshold parameter, which MUST be a single digit between "2" and "9", or the digit "0". | ||
| *** If the threshold parameter is "0" then the share index, defined below, MUST have a value of "s" (or "S"). | ||
| ** A threshold digit (also called ''k'' in the codex book), which MUST be a single digit between "2" and "9", or the digit "0". This digit encodes the threshold (the number of shares required for recovery), where threshold 1 is encoded as "0" for unshared secrets, and thresholds 2-9 are encoded as digits "2"-"9" for shared secrets. | ||
| *** If the threshold digit is "0" then the share index, defined below, MUST have a value of "s" (or "S"). |
There was a problem hiding this comment.
Digit vs parameter is probably bikeshedding
There was a problem hiding this comment.
Agreed. And I don't think "digit" is the right term for a bech32 character anyway.
bip-0093.mediawiki
Outdated
|
|
||
| For an unshared secret, the threshold parameter (the first character of the data part) is ignored (beyond the fact it must be a digit for the codex32 string to be valid). | ||
| We recommend using the digit "0" for the threshold parameter in this case. | ||
| For an unshared secret, the threshold digit (the first character of the data part, also called ''k'') is ignored (beyond the fact it must be a digit for the codex32 string to be valid). |
There was a problem hiding this comment.
Even if @apoelstra believes BIP93 should clarify the Codex32 book uses a different term k for the threshold parameter. We definitely should not keep repeating both nomenclatures.
bip-0093.mediawiki
Outdated
| For an unshared secret, the threshold parameter (the first character of the data part) is ignored (beyond the fact it must be a digit for the codex32 string to be valid). | ||
| We recommend using the digit "0" for the threshold parameter in this case. | ||
| For an unshared secret, the threshold digit (the first character of the data part, also called ''k'') is ignored (beyond the fact it must be a digit for the codex32 string to be valid). | ||
| We recommend using the digit "0" for the threshold digit in this case, which encodes a threshold of 1 (no sharing). |
There was a problem hiding this comment.
Technically share index "s" denotes a threshold 1unshared secret as the threshold parameter is ignored in this case and it should stay that way for interoperability.
| ===Recovering Master Seed=== | ||
|
|
||
| When the share index of a valid codex32 string (converted to lowercase) is not the letter "s", we call the string an codex32 share. | ||
| The first character of the data part indicates the threshold of the share, and it is required to be a non-"0" digit. |
There was a problem hiding this comment.
This line is correct. Read the line above: when the share index is not "s". The threshold parameter cannot be "0" in that case, so we know that for a codex32 share, the threshold parameter is required to be 2-9.
|
It sounds like it is contested that this is an improvement, even if there may be some need for clarification here. Please update the PR and mark any review comments as resolved as you address them. |
|
Please remove the |
|
@apoelstra: The last sentence of the Abstract of the currently published version of BIP 93 reads:
@Lil-Duckling-22: Please incorporate the requested changes. |
5 participants
Update Abstract to align with specification: threshold can be 0 for unshared secret or 2-9 for shares.
Apparently previous "between 1 and 9" range appears inconsistent with the detailed spec (threshold=1 isn't valid in codex32).