Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
EIP-0042 Multi-Signature Wallet #88
base: master
Are you sure you want to change the base?
EIP-0042 Multi-Signature Wallet #88
Changes from all commits
3fc9ea1
4596740
ac31fee
d89ff56
b416ada
8f5caab
6304ebb
7667389
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Differs from EIP-11, see below
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.
Agree, if EIP-11 is not suitable "as-is", then please explain why and if motivated, then EIP-11 should be updated. Then this EIP should follow EIP-11.
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.
this format is used in the node, sigma-rust and other libs, see /wallet/generateCommitments.
I'm not sure why EIP-11 is not following this format.
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.
This format differs from the actual format in EIP-11 (index/position, pubkey/pk, a/commitment). Is EIP-11 outdated?
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.
Why does the last wallet create a "partially signed transaction" and not a fully signed transaction if it already has N commitments? Or does this sentence apply until N-1 commitments are collected?
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.
see https://github.com/ergoplatform/eips/blob/9906f403f61d8592ab955494cc4310563f4067c6/eip-0011.md#signing-procedure.
It's a two-round protocol in which the parties generate commitments in the first round. The last party is able to partially sign (he has only one of the keys so he cannot fully sign) and then, each of them signs the transaction partially and sends the generated signature to another party. This process continues until the signature is completed.
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.
What means simulated? This is only the list of missing keys, which can be determined by the wallet address. Why do we need it?
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.
see https://storage.googleapis.com/ergo-cms-media/docs/ErgoScript.pdf, Appendix A2. Proving.
Those signers that do not participate in signing will be simulated. See above for details.
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.
There is a mistake.
The "Collection Element Data Type (SigmaProp)" is not saved.
Please check ConcreteCollectionSerializer and ValueSerializer.
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.
I'm not sure if I got it, please elaborate.
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.
On your picture, the "Collection Element Data Type (SigmaProp)" shown under "Repeated for each public key". This is not correct. That 08 value is stored only once. And you already showing it as "Collection Data Type (SigmaProp)"
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.
according to the doc, it seems you're right since Expression Serializations requires opCode and body; however, in the tests I'm getting parsing error if I eliminate those types and surprisingly it's working when type is added.