Signature Aggregation #210

bvohaska · 2019-03-22T16:31:36Z

Initial changes to Signature spec

Updated Sign, Verify, Recover
Added aggregate
Added a section for Recovery
Added a subsection for BLS signature public key recovery

signatures.md

…n Sign and Aggregate. Added BLS12-381 parameters.

whyrusleeping · 2019-03-26T02:15:23Z

@bvohaska what sort of review are you looking for right now?

bvohaska · 2019-03-26T03:56:48Z

@bvohaska what sort of review are you looking for right now?

@whyrusleeping: I’m looking for any feedback to make sure this spec is heading in the right direction. No deep review needed yet. Should have most or all of this spec ready for deep review by 28 March.

_{Sent with GitHawk}

…edMessage. Added exposition for Aggregates.

…nged Signature interface methods. Removed redundant references to Address since address is contained in SignedMessage. Reworked the references section. REmoved libsecp256k1 wire format b/c it is too complicated; added a reference to the authoritative code. :-(

bvohaska · 2019-03-27T22:07:00Z

@whyrusleeping added a very rough description for BLS public key recovery based on this. Please update with corrections.

This update should represent the last update for signature.md. All reviewers should feel free to commence a deep review on that file.

…nature must implement the Signature interface.

whyrusleeping · 2019-03-29T20:36:38Z

signatures.md

        //
-        Verify(m Messgage, pk PublicKey, sig SignatureBytes) (valid bool, err error)
+  			VerifyAggregate(smsgs SignedMessgage,  pks []PublicKeys) (valid bool, err error)


are there multiple signed messages here? also does the signature get passed in? or how does this work?

Updating name "smgs" --> "smsg" to reduce confusion. Any SignedMessage containing a signature aggregate will contain the signature aggregate as well as all messages contained therein. So we only need the one SignedMessage as input.

Setting aside golang syntax, suppose there is X = SignedMessage. To verify a signature aggregate, VerifyAggregate will perform something like:

For all messages m_i in X.Msgs, h_i = hash2curve(m_i)

Compute S = deserialize(X.SignatureBytes)

Compute T = e(h_1, pk_i)*...*e(h_n, pk_n)

If T = e(S, P2) => valid

signatures.md

whyrusleeping · 2019-03-29T20:40:34Z

signatures.md


-Currently, Filecoin uses secp256k1 signatures to fulfill the above interface. All signatures on messages, blocks, and tickets currently use the same scheme and format.
+```go
+Type SignedMessage Struct {


where does this type actually get used? This is not the signed message that gets sent over the network for message propogation, its something else, right?

There should be only one SignedMessage --> https://github.com/filecoin-project/go-filecoin/blob/master/types/signed_message.go#L22-L28

If there are more than one, then we should do some disambiguation.

signatures.md

whyrusleeping · 2019-03-29T20:57:59Z

good detail on the base cryptographic primitives (are there RFCs for some of that? seems like stuff that should go in the standard)

A bit more still needed before completion:

Spec for how message signatures are aggregated by the miner when putting together the block
spec for how a block with aggregated signatures gets verified
- Where do the public keys come from here? (they come from the chain somehow, but that needs to be detailed)
- The account actor needs to be upgraded to hold public keys for bls based accounts
What does the array of signed messages in the block look like now that some of the messages have had their signatures aggregated?
How exactly do we disambiguate signatures?
add the signature type to the data-structures doc, along with pointers to how things are serialized.

probably more questions, but once the above is done, let's get dev to review. A sign-off from them is what we're after.

… 'smgs'--> 'smsg', commented out the Sign method for the Signature interface as it will need to be a method on a privateKey

dignifiedquire

Overall nice shape 👍

I haven't found any documentation about how hashing is done, and what exactly is hashed in terms of bytes. Maybe this is supposed to be defined somewhere else, but needs to be there eventually

signatures.md

dignifiedquire · 2019-04-02T12:25:53Z

signatures.md

+- Avg. processing power available to a CPU 
+  - Affects ability to verify signature aggregates
+  - Each signature or collection of signatures should be validated with no impact to the consumption of the chain
+- `Gas` cost - `Gas(ECDSA) > Gas(BLS)`


please clarify which operations are more costly than others, eg Gas(ECDSA_Sign)

I don't have this information. Ping @whyrusleeping : can you answer this?

signatures.md

frrist

This is really thorough, nice work!

I have a couple questions about the assumptions we are making wrt addresses using protocol 0.

signatures.md

frrist · 2019-04-02T16:56:02Z

signatures.md

+Input: Address
+Output: "BLS", "ECDSA", or "INVALID"
+
+   1.  If Address.protocol = 0 output "BLS" and stop


I believe this could be ECDSA, BLS or INVALID. Protocol 0 indicates the address represents an ID and ID's are issued to for all actors. Some discussion was had here about creating a mapping from ID to PublicKey, I think we will want to consider this here.

I see you have covered this case here: #123 (comment)

Does Address.protocol specify 0? If so, fetch actor with ID = Address.payload, return BLS or ECDSA. <--this method needs to be specified in actors.md and should distinguish between BLS and ECDSA signatures.

frrist · 2019-04-02T17:23:05Z

signatures.md

+Output: "Aggregate" or "INVALID"
+
+	 1.  If SignedMessage.Address[i].protocol != {0 OR 3} for all i output "INVALID" and stop
+	 2.  If len(SignedMessage.Msgs) > 1 output "Aggregate" else output "INVALID"


Would the case len(SignedMessage.Msgs) == 1 indicate the the SignedMessage contains a ECDSA signature?

What about the case of no addresses and one message? Is an aggregate of one explicitly invalid? While that's what the pseudocode implied, explicit clarification would help.

What about the case of multiple addresses and multiple messages? I'm guessing this means an aggregate (but should it check the lengths are equal)? An enumeration of all possible cases of {1, many}-messages ⨯ {0, many}-addresses would help (perhaps in the prose above).

@frrist: yes. ECDSA signatures can't be aggregated in this scheme so if we encounter them we should error. Maybe "INVALID" is not the right wording. We should probably specify input validation. Alex alludes to this below.

@anorth: Good point. Request someone from go-filecoin to work on this (input validation).

Added note about ECDSA not being aggregatable in the "Aggregating Signed Messages" section

I would love us to work on input validation, but we need you to specify what inputs should be valid. Please enumerate all combinations that are semantically valid and explicitly state that other combinations are meaningless.

anorth · 2019-04-04T00:23:32Z

signatures.md

 }
 ```

+### Signed Message
+
+This is the container for a signature that combines a message with an address and signature. 


I'm confused by the plurality of addresses and messages in the structure given this description.

I understand the use case for multiple addresses, but what is the case for multiple different messages? This seems like redundant data in the case of multiple signers of an identical message.

A brief outline of use cases and capabilities at the top of this doc would help greatly if my understanding of the motivations and context is lacking.

I'm note sure I quite understand but I've reworked the description. Agree it is very confusing. Will push. Let me know if that answers your questions.

I still don't understand why there are multiple messages in the container. The comment uses the singular "a message and an address" which disagrees with the code directly below. Please clarify the intended role of a SignedMessage. E.g. you could write:

A container for one or multiple messages and an aggregation of the signatures of those messages by corresponding addresses. In the case of a single message and address, the signature is a vanilla signature, not an aggregate. In the case of multiple messages, the signature is assumed to be an aggregate. See *Determining signature type* below for more detail.

But I don't know whether that is in fact correct or complete.

anorth · 2019-04-04T00:32:25Z

signatures.md

-
-}
+If there is more then one message in `SignedMessage.Msgs` then the `SignedMessage` is assumed to be a signature aggregate. If `Address` is empty then `Verify` will assume the `SignedMessage` is a signature aggregate. The procedure below uses `len()` to indicate a function that returns the number of elements in an array.
+


As a practical matter, I recommend an explicit representation of the "type" of signed message rather than assuming from the number of messages and/or addresses. An explicit byte identifying the semantics will simplify future changes and make code easier to understand and more likely to be correct.

@whyrusleeping: ^ . It would make things easier to understand but would cost a bit or two on-chain. What do you think?

anorth · 2019-04-04T00:42:20Z

signatures.md

+Output: "Aggregate" or "INVALID"
+
+	 1.  If SignedMessage.Address[i].protocol != {0 OR 3} for all i output "INVALID" and stop
+	 2.  If len(SignedMessage.Msgs) > 1 output "Aggregate" else output "INVALID"


What about the case of no addresses and one message? Is an aggregate of one explicitly invalid? While that's what the pseudocode implied, explicit clarification would help.

What about the case of multiple addresses and multiple messages? I'm guessing this means an aggregate (but should it check the lengths are equal)? An enumeration of all possible cases of {1, many}-messages ⨯ {0, many}-addresses would help (perhaps in the prose above).

…hods for signature interface. Added note indicating ECDSA signatures are not aggregatable.

signatures.md

whyrusleeping · 2019-04-11T02:17:16Z

mining.md

@@ -107,7 +107,7 @@ Chain selection is a crucial component of how the Filecoin blockchain works. Eve

 ### Block Validation

-The block structure and serialization is detailed in [the datastructures spec](data-structures.md#block). Check there for details on fields and types.
+The block structure and serialization is detailed in [the datastructures spec](data-structures.md#block). Check there for details on fields and types. All references to `VaidateSignature` must implement the `Signature` interface found [here](https://github.com/filecoin-project/specs/blob/master/signatures.md).


This addition is confusing. Why not just say that signature validation is covered in detail in the signature spec?

This is a good point. I've been mulling over how to best represent that ValidateSignature must use the verification method in signatures.md. The current phrasing is confusing b/c it should read: "Block implements the Signature interface" and ValidateSignature --> Block.Verify(SignedMessage_Block, PublicKey). in this case, the internal block messages would be represented as a single value (hashed), SignatureBytes would be the actual signature of the message and Address would be the address of the block proposer.

we also need a place that implements verification of the aggregated signatures, e.g. a method like VerfiyAggregates([]Message, SignatureBytes) bool for use inside the block validation.

bvohaska · 2019-04-15T17:32:14Z

@anorth, @whyrusleeping: This PR is suffering a bit of scope creep. Let's close it out and push to master. I'd like to ship this so we can move a bit faster. We can create a new PR for other concerns captured in this PR. I believe most of the concerns are captured in this GH issue.

whyrusleeping · 2019-04-15T19:08:30Z

@bvohaska I don't think its scope creeping. The scope of signature aggregation should be every change needed to the protocol to support it. I don't want to merge a partial spec change which would leave the spec in a weird place here.

anorth

I have unresolved two questions about the specific semantics of the SignedMessage structure.

I would also like an explicit call, perhaps from @whyrusleeping, on an explicit type identifier to distinguish possible interpretations. If that call is "no", then the exhaustive enumeration of valid and invalid combinations is important to understanding the avenues open for change in the future.

Beyond that, this LGTM, but I expect my approval isn't sufficient to merge.

anorth · 2019-04-16T04:21:24Z

signatures.md

 }
 ```

+### Signed Message
+
+This is the container for a signature that combines a message with an address and signature. 


I still don't understand why there are multiple messages in the container. The comment uses the singular "a message and an address" which disagrees with the code directly below. Please clarify the intended role of a SignedMessage. E.g. you could write:

A container for one or multiple messages and an aggregation of the signatures of those messages by corresponding addresses. In the case of a single message and address, the signature is a vanilla signature, not an aggregate. In the case of multiple messages, the signature is assumed to be an aggregate. See *Determining signature type* below for more detail.

But I don't know whether that is in fact correct or complete.

anorth · 2019-04-16T04:24:11Z

signatures.md

+Output: "Aggregate" or "INVALID"
+
+	 1.  If SignedMessage.Address[i].protocol != {0 OR 3} for all i output "INVALID" and stop
+	 2.  If len(SignedMessage.Msgs) > 1 output "Aggregate" else output "INVALID"


I would love us to work on input validation, but we need you to specify what inputs should be valid. Please enumerate all combinations that are semantically valid and explicitly state that other combinations are meaningless.

mhammersley · 2019-04-16T17:55:14Z

suggestion: have a set of diagrams describing the constituent elements as well as the process flow with concise, precise definitions

…ge to better comply with current understanding. Moved SignedMessage to data-structures.md; updated Ticket definition to comply with SignedMessage.

…ts in the PR and sync meetings

bvohaska · 2019-04-17T01:25:17Z

Moved content of this comment to the tracking issue here

bvohaska · 2019-04-17T01:20:53Z

data-structures.md

-    GasPrice Integer
-    GasLimit Integer
+
+	GasPrice Integer


@whyrusleeping, @dignifiedquire: is there a canonical description for Integer? In other words, what exactly do we mean by integer here? Is this a 32 byte binary quantity? Perhaps it's a bigInt in form LEB128?

Do we know the size of these Integers?

bvohaska · 2019-04-17T01:22:07Z

data-structures.md

+type Ticket struct {
+
+	// The VDF Result is derived from the prior ticket in the ticket chain
+	VDFResult BigInteger


@sternhenri, @ZenGround0: Do we have a language-independent description of BigInteger here? What is the binary representation? Maybe we could point to a spec that defines this for us.

That is what LEB128 is for

Do we have a size for these bigInts?

They are variably length sized, to support arbitrary numbers. But if you have a max range you want to represent then I can tell you the size.

Understood. Someone will have to update the spec comments to expound on this. As for the typing, we are using inconsistent type nomenclature for big integers. It would be nice to go through and standardize these. So far I've seen Integer, BigInteger, BigInt; they no doubt mean the same thing --> LEB128 encoded integers of arbitrary size but we should specifically call that out here.

If the spec owners acknowledge that this is what they mean when they write an element from the list above, I'll go ahead and change it. Don't want to misinterpret the authors intent.

bvohaska · 2019-04-17T01:40:45Z

data-structures.md


 	// MessageReceipts is a set of receipts matching to the sending of the `Messages`.
-    // TODO: should be the same type of merkletree-list thing that the messages are
+	// TODO: should be the same type of merkletree-list thing that the messages are
 	MessageReceipts []MessageReceipt


@whyrusleeping: what are MessageReceipts?

They are defined below and store the result of applying the message

I see the struct definition. Do we have any exposition as to why we need them? Given that they are an on-chain quantity, I would claim that it's important to declare exactly why we have them. This would be the best place to put that explanation.

dignifiedquire · 2019-04-18T12:08:18Z

data-structures.md

+
+	// ElectionProof is a signature over the final ticket that proves this miner
+	// is the leader at this round
+	ElectionProof SignatureBytes


Is it defined somewhere what kind of signature this is cc @sternhenri

^ I took a guess at this. Previously, it read Signature. Will need @sternhenri to verify this is a BLS signature.

dignifiedquire · 2019-04-18T12:11:38Z

data-structures.md

 ## Block

 A block represents an individual point in time that the network may achieve consensus on. It contains (via merkle links) the full state of the system, references to the previous state, and some notion of a 'weight' for deciding which block is the 'best'.

 ```go
 // Block is a block in the blockchain.
 type Block struct {
+
 	// Miner is the address of the miner actor that mined this block.
 	Miner Address

 	// Tickets are the winning ticket that were submitted with this block.
 	Tickets []Ticket


Could we aggregate the signatures on the tickets as well?

If the EC spec can be updated to support it, I don't see why not. We'll need to make sure that the EC spec is using SignedMessages and the Signature interface.

^ thoughts @sternhenri?

dignifiedquire · 2019-04-18T12:13:48Z

data-structures.md

+
+  // BLSMessages is the set of messages with associated BLS signatures contained
+  // in BLSSignatureAggregate.
+  BLSMessages []Message


In general I like this structure, but we are now unable to present an ordering of messages in the form of

- bls message - ecdsa message - bls message

In regards to execution order.

I would argue this is fine, and miners are simply asked to process these not together, but first one kind, then the other kind. But this is an explicit design decision that might have other implications.

bvohaska · 2019-04-22T16:39:41Z

@whyrusleeping: from mining.md (https://github.com/filecoin-project/specs/blob/master/mining.md#block-validation), it doesn't look like on-chain message signatures are validated during block validation. Maybe I'm missing something?

sternhenri · 2019-11-05T19:47:08Z

outdated

init changes to signature spec to comply with Issue #123

79cfdc4

whyrusleeping reviewed Mar 22, 2019

View reviewed changes

signatures.md Outdated Show resolved Hide resolved

whyrusleeping reviewed Mar 24, 2019

View reviewed changes

signatures.md Outdated Show resolved Hide resolved

added minor changes requested by @whyareusleeping. Added exposition o…

f343b58

…n Sign and Aggregate. Added BLS12-381 parameters.

changed input parameters for Signature.Verify. Changed fields in Sign…

c397188

…edMessage. Added exposition for Aggregates.

bvohaska changed the title ~~init changes to signature spec to comply with Issue #123~~ (Draft Spec) Signature Aggregation Mar 27, 2019

bvohaska changed the title ~~(Draft Spec) Signature Aggregation~~ Signature Aggregation Mar 27, 2019

bvohaska mentioned this pull request Mar 27, 2019

(Draft Spec) Signing & Signature Aggregation #123

Closed

13 tasks

bvohaska added 2 commits March 27, 2019 12:01

added ECDSA exposition. Added information for BLS signature recovery.

943183c

bvohaska requested review from dignifiedquire and anorth March 27, 2019 22:07

added comments to actors and mining specs that state that ValidateSig…

9cd1b6a

…nature must implement the Signature interface.

whyrusleeping reviewed Mar 29, 2019

View reviewed changes

signatures.md Outdated Show resolved Hide resolved

whyrusleeping reviewed Mar 29, 2019

View reviewed changes

signatures.md Outdated Show resolved Hide resolved

whyrusleeping reviewed Mar 29, 2019

View reviewed changes

signatures.md Show resolved Hide resolved

whyrusleeping requested a review from frrist March 29, 2019 21:00

added minor cosmetic changes: removed go-filecoin references, changed…

8f7eed4

… 'smgs'--> 'smsg', commented out the Sign method for the Signature interface as it will need to be a method on a privateKey

dignifiedquire reviewed Apr 2, 2019

View reviewed changes

updated signature.md to include corrections from dignifiedinquire

3b0cc40

frrist reviewed Apr 2, 2019

View reviewed changes

anorth reviewed Apr 4, 2019

View reviewed changes

bvohaska added 2 commits April 9, 2019 12:27

formatting updates. Added references requested in the PR.

e652297

reworked SignedMessage description. Added marshall and Unmarshall met…

81b7c16

…hods for signature interface. Added note indicating ECDSA signatures are not aggregatable.

bvohaska commented Apr 9, 2019

View reviewed changes

signatures.md Outdated Show resolved Hide resolved

whyrusleeping reviewed Apr 11, 2019

View reviewed changes

anorth reviewed Apr 16, 2019

View reviewed changes

bvohaska added 2 commits April 16, 2019 21:06

moved Tickets definition to data-structures.md; redefined SignedMessa…

ee3d49b

…ge to better comply with current understanding. Moved SignedMessage to data-structures.md; updated Ticket definition to comply with SignedMessage.

reworked methods in the Signature interface to better reflect coommen…

8e71e61

…ts in the PR and sync meetings

bvohaska commented Apr 17, 2019

View reviewed changes

added message breakdown in the Block struct

27fc4b8

bvohaska commented Apr 17, 2019

View reviewed changes

dignifiedquire reviewed Apr 18, 2019

View reviewed changes

added description of BLS aggregation for block creation

202ef41

pooja mentioned this pull request Jul 3, 2019

TODO for July 23 #378

Closed

19 tasks

pooja mentioned this pull request Jul 30, 2019

TODO for August 6 #418

Closed

18 tasks

pooja mentioned this pull request Aug 27, 2019

TODO for September 3 #460

Closed

22 tasks

sternhenri closed this Nov 5, 2019

nicola deleted the feat/sig-aggregation branch November 29, 2019 02:09


		}
		If there is more then one message in `SignedMessage.Msgs` then the `SignedMessage` is assumed to be a signature aggregate. If `Address` is empty then `Verify` will assume the `SignedMessage` is a signature aggregate. The procedure below uses `len()` to indicate a function that returns the number of elements in an array.

Signature Aggregation #210

Signature Aggregation #210

Conversation

bvohaska commented Mar 22, 2019 • edited by pooja Loading

whyrusleeping commented Mar 26, 2019

bvohaska commented Mar 26, 2019

bvohaska commented Mar 27, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

whyrusleeping commented Mar 29, 2019

dignifiedquire left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

frrist left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bvohaska commented Apr 15, 2019

whyrusleeping commented Apr 15, 2019

anorth left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mhammersley commented Apr 16, 2019

bvohaska commented Apr 17, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bvohaska Apr 22, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bvohaska commented Apr 22, 2019 • edited Loading

sternhenri commented Nov 5, 2019

bvohaska commented Mar 22, 2019 •

edited by pooja

Loading

bvohaska commented Mar 27, 2019 •

edited

Loading

bvohaska Apr 22, 2019 •

edited

Loading

bvohaska commented Apr 22, 2019 •

edited

Loading