Skip to content
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

Modify 0x01 short format to fit multisig lock standard. #141

Merged
merged 5 commits into from
Nov 6, 2019

Conversation

CipherWang
Copy link
Contributor

No description provided.

@jjyr
Copy link
Contributor

jjyr commented Oct 26, 2019

LGTM

S | R | M | N | Pubkey1 | Pubkey2 | ...
```

Where S/R/M/N are four single byte unsigned integers, ranging from 0 to 255, and Pubkeys are SECP256K1 public keys. S is format version, currently fixed to 0. M/N means the user must provide M of N signatures to unlock the cell. And R means the provided signatures at least match the first R items of the Pubkey list.
Copy link
Member

Choose a reason for hiding this comment

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

What's the use of S?

Copy link
Contributor

Choose a reason for hiding this comment

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

In the original design S is a reserved field for future use, occupied 1 byte.

Copy link
Member

Choose a reason for hiding this comment

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

For what kind of use? If there's no explicit use I prefer to remove it. We can use a new code_hash_index if we want to add a new multisig format in the future.

Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure yet, it brings some extensibility,
we may use it to store flags to enable the new features. Or use it as a version to allow compatible changes in the future.

Of cause, use code_hash_index can also achieve the above. what do you think? @CipherWang

Copy link
Contributor

Choose a reason for hiding this comment

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

The code nervosnetwork/ckb-system-scripts#60 is supposed to be freeze, we decide to do not touch the current code, we still keep the S.

jjyr
jjyr previously approved these changes Oct 30, 2019
janx
janx previously approved these changes Oct 30, 2019
@CipherWang CipherWang dismissed stale reviews from janx and jjyr via dfdfce8 November 1, 2019 01:04
@doitian doitian merged commit 54247f1 into nervosnetwork:master Nov 6, 2019
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.

6 participants