Add spec for proofs #89

MavenRain · 2023-01-03T15:31:00Z

No description provided.

hansl · 2023-01-03T18:00:45Z

spec/cddl/many.cddl

@@ -39,6 +39,11 @@ request = COSE_Sign<{
    ; Attributes. An optional list of request attributes defined in the spec. See
    ; the list of all attributes in this repo.
    ? 8 => [ * attribute ],
+
+    ; Flag.  When set, a proof of what was requested will be given, if possible.


I think an attribute here would be better, as:

Attributes are meant to be optionally implemented, with clear rules when implementation is missing (request handlers reject them, response handlers ignore them) which actually makes sense here,

Adding fields to messages is a bit like releasing a new version of HTTP. Attributes are the mechanism to enhance them (similar to HTTP headers[1]),

Servers can advertise which attributes they support, but with this ALL servers should implement proofs, which is unreasonable.

So I'd rather have a system similar to async support; a request attribute, a response attribute and a server attribute that indicates support for those. This way servers can advertise they support proofs, clients can ignore proofs, and if proof is requested the server should bail out as it cannot provide it, and client can then re-request without the attribute if it chooses so.

[1] there is an argument that HTTP headers are different as they are optional while attributes require handling or fail, but in this specific case it is actually beneficial.

Acknowledged . . . will refactor this into a proof attribute

hansl · 2023-01-03T18:01:37Z

spec/cddl/many.cddl

+
+    ; Proof.  If requested, this is the set of steps constituting a proof of existence
+    ; of what was requested in the state of the Merkle tree.
+    ; See https://github.com/liftedinit/merk/blob/develop/docs/algorithms.md#binary-format for


I'd like to put the thoughts of this document into a separate specification document and not rely on implementation specific documentation.

Note that using specific implementations is not something the spec should recommend or even mention.

OK, I can put the binary structure into a separate adoc

fmorency

Nice, thanks! Minor changes.

@hansl is there an issue somewhere tracking this feature?

fmorency · 2023-01-03T22:22:59Z

attributes/network/11_proof.adoc

@@ -0,0 +1,32 @@
+= Proof Attribute (#11)


This attribute number is already taken in #73, and @hansl has taken #73 in #86.

You should probably use 14!

fmorency · 2023-01-03T22:24:44Z

attributes/network/11_proof.adoc

+Interpreting these operations as acting on a stack, the first three operations listed
+can be interpreted as pushing data onto the stack.  In the parent operation,
+two items are popped from the stack, and a new item with the second item as the
+left child of the first item is pushed onto the stack.  In the child operation,
+two items are popped from the stack, and a new item with the first item as the right child
+of the second item is pushed back onto the stack.


I like the way they present it in https://github.com/liftedinit/merk/blob/develop/docs/algorithms.md#binary-format

Their example makes everything very clear.

fmorency · 2023-01-03T22:25:07Z

attributes/network/cddl/11_proof.cddl

+
+    ; The value
+    1 => node-value
+}


Add EOF EOL

fmorency · 2023-01-03T22:26:40Z

spec/cddl/many.cddl

I'm going to reiterate this. It's nicer for diffs to have a newline at the end of a file.

I'm also okay if you want to add an .editorconfig or .env file to this repo.

fmorency · 2023-01-03T22:29:25Z

attributes/network/11_proof.adoc

@@ -0,0 +1,32 @@
+= Proof Attribute (#11)


Can you add a link to the CDDL file? See other .adoc for examples.

…4, and proof variants fixed to be distinguishable according to tagged array convention

…sentation

hansl · 2023-01-04T19:32:48Z

attributes/network/cddl/14_proof.cddl

+node-hash = [2, bstr]
+
+; The key-value pair of a proof
+key-value-pair = [3, (0 => bstr, 1 => bstr)]


Suggested change

key-value-pair = [3, (0 => bstr, 1 => bstr)]

key-value-pair = [3, bstr, bstr]

hansl · 2023-01-05T21:01:26Z

attributes/network/14_proof.adoc

+
+This is a uint representing the hasher to be used in constructing proofs.
+
+* 0 - merk default hash (currently blake3)


Should point to the Hash Scheme section of this document. I have a feeling we'll have more than one later too, so if you want to create a new document and point to it that'd be even better.

hansl · 2023-01-05T21:01:34Z

attributes/network/14_proof.adoc

+
+* 0 - merk default hash (currently blake3)
+
+* 1 - TBD


hansl · 2023-01-05T21:05:44Z

attributes/network/14_proof.adoc

+
+* 1 - TBD
+
+== Hash Scheme


Describing a Merkle tree and how to calculate the proof is fine, but we should separate the Merkle tree part from the description of the hashing itself.

@hansl I've moved this part to a separate doc and referenced. To clarify, is it right that the attribute id says use the hash scheme, or does it say use what merk currently uses as the hash function (blake3)?

hansl · 2023-01-05T21:06:41Z

spec/cddl/many.cddl

I'm going to reiterate this. It's nicer for diffs to have a newline at the end of a file.

I'm also okay if you want to add an .editorconfig or .env file to this repo.

… kv_hashes and node hashes

fmorency

Thanks, @MavenRain!

attributes/network/14_proof.adoc

fmorency · 2023-01-05T21:27:58Z

attributes/network/14_proof.adoc

+This is a uint representing the hasher to be used in constructing proofs.
+
+* 0 - merk default hash (currently blake3)
+
+* 1 - TBD


Is the attribute mandatory?

attributes/network/14_proof.adoc

fmorency · 2023-01-05T21:44:27Z

attributes/network/14_proof.adoc

+
+This is a uint representing the hasher to be used in constructing proofs.
+
+* 0 - merk default hash (see xref:../../spec/proof_hash_scheme.adoc.)


I would not put any implementation detail here.

We should be able to query the server and retrieve the hashes it supports.

@fmorency I believe this contradicts what @hansl just requested.

@hansl WDTY?

attributes/network/cddl/14_proof.cddl

fmorency · 2023-01-05T21:48:39Z

spec/proof_hash_scheme.adoc

+Each node contains a "kv hash", which is the hash of a prefix concatenanted with the nodes key and its value.
+The hash of the node is the hash of a distinct prefix, the kv hash, and the hash of each of the left and right child nodes.
+
+```
+kv_hash = H(0x00, key, value)
+node_hash = H(0x01, kv_hash, left_child_hash, right_child_hash)
+```


This is not exactly true, as our merk fork doesn't implement the second preimage attack mitigation, i.e., it doesn't have the 0x00 and 0x01 prefixes.

I would try to keep the specification general. Some of our users might use other storage implementations.

@fmorency My understanding is that we're to use the attribute-arg to allow for the ability to change the implementation, and this describes the default implementation. Also, I believe @hansl intends for us to incorporate the prefix changes that are currently in the upstream repo.

fmorency

Some comments/suggestions!

fmorency · 2023-01-10T18:53:47Z

attributes/response/3_proof.adoc

@@ -0,0 +1,17 @@
+= Proof Attribute (#3)


Suggested change

= Proof Attribute (#3)

= Proof Response Attribute (#3)

fmorency · 2023-01-10T18:54:54Z

attributes/response/3_proof.adoc

+
+== Attribute Argument
+
+This is a uint representing the hasher to be used in constructing proofs.


Suggested change

This is a uint representing the hasher to be used in constructing proofs.

This is a `uint` representing the hasher used in constructing the proofs.

fmorency · 2023-01-10T18:55:11Z

attributes/response/3_proof.adoc

+
+This is a uint representing the hasher to be used in constructing proofs.
+
+* 0 - merk default hash (see xref:../../spec/proof_hash_scheme.adoc.)


Suggested change

* 0 - merk default hash (see xref:../../spec/proof_hash_scheme.adoc.)

* 0 - `merk` default hash (see xref:../../spec/proof_hash_scheme.adoc.)

attributes/network/cddl/14_proof.cddl

fmorency · 2023-01-10T18:57:31Z

attributes/request/3_proof.adoc

@@ -0,0 +1,7 @@
+= Proof Request Attribute (#3)


What will go here?

hansl · 2023-01-12T21:19:18Z

attributes/response/cddl/3_proof.cddl

@@ -0,0 +1,17 @@
+; Proof.  If requested, this is the set of steps constituting a proof of existence of what was requested in the state of the Merkle tree.
+proof@response-attribute-arg = [ + ( node-hash / key-value-hash / key-value-pair / parent / child ) ]


The response attribute should be a map, it should contain at least 2 values:

the root hash, and

the proof itself.

Add an optional block height for now, but make it a attribute-related-index.

Suggested change

proof@response-attribute-arg = [ + ( node-hash / key-value-hash / key-value-pair / parent / child ) ]

proof@response-attribute-arg = {

; Root application hash.

0 => bstr,

; The proof operations.

1 => proof,

; Extensible attribute related indices for extra information (implementation specific)

* attribute-related-index => (),

}

proof = [ + ( node-hash / key-value-hash / key-value-pair / parent / child ) ]

And then in a separate CDDL 3_proof/0_blockchain.cddl file, get the following in:

proof@response-attribute-arg //= { ; Block height. [3, [0, 0]] => uint }

hansl · 2023-01-12T21:21:39Z

spec/proof_hash_scheme.adoc

+The hash of the node is the hash of a distinct prefix, the kv hash, and the hash of each of the left and right child nodes.
+
+```
+kv_hash = H(0x00, key, value)


Missing key and value length. This is closer:

Suggested change

kv_hash = H(0x00, key, value)

kv_hash = H(0x00, key.length, key, value.length, value)

hansl · 2023-01-12T21:21:46Z

spec/proof_hash_scheme.adoc

+
+```
+kv_hash = H(0x00, key, value)
+node_hash = H(0x01, kv_hash, left_child_hash, right_child_hash)


Same as above.

Actually, the node hash does not have the lengths in the hash.

…fined in response cddl

attributes/network/14_proof.adoc

Co-authored-by: Hans Larsen <hans@larsen.online>

Outdated

Add spec for proofs

ae6cb3d

MavenRain requested review from hansl and fmorency January 3, 2023 15:31

MavenRain marked this pull request as ready for review January 3, 2023 16:15

hansl reviewed Jan 3, 2023

View reviewed changes

Refactoring to proof attribute

c5e173b

fmorency requested changes Jan 3, 2023

View reviewed changes

MavenRain added 3 commits January 4, 2023 07:51

Proof response given number 3, proof network attribute given number 1…

e0b62bc

…4, and proof variants fixed to be distinguishable according to tagged array convention

Add attribute-arg to proof attribute, and change key-value-pair repre…

1c4f105

…sentation

Add example proof construction to adoc

71e0c5b

hansl reviewed Jan 4, 2023

View reviewed changes

MavenRain added 4 commits January 4, 2023 12:01

Key-value-pair change

f67e687

Reorganizing and creating hash scheme section

c97b7ff

Add text for hash scheme

c583207

hash attribute id comment edit

2b314cb

MavenRain requested review from fmorency and hansl January 5, 2023 01:59

hansl requested changes Jan 5, 2023

View reviewed changes

MavenRain added 4 commits January 5, 2023 13:38

Add newlines at end of file adn move hash scheme doc to separate file

521b459

Fix typo

a3331d9

More newlines in files

53abb81

Make clear the hash function used in the hash scheme for constructing…

e2fc0c3

… kv_hashes and node hashes

fmorency reviewed Jan 5, 2023

View reviewed changes

Swapping contents of network proof adoc and response proof adoc

486a3eb

MavenRain requested a review from hansl January 6, 2023 18:20

Add stub for proof request attribute adoc

333f579

MavenRain requested a review from fmorency January 10, 2023 16:45

fmorency previously requested changes Jan 10, 2023

View reviewed changes

Rearrange items in spec to fit the attribute pattern for proofs

23c2995

hansl reviewed Jan 12, 2023

View reviewed changes

Add newline

aea2dc3

hansl reviewed Jan 12, 2023

View reviewed changes

MavenRain added 3 commits January 13, 2023 15:30

Complete revisions according to last set of review comments

12d0bd4

Added newline

09bf877

Another newline

7bce59f

MavenRain requested a review from hansl January 14, 2023 00:58

MavenRain added 12 commits January 13, 2023 17:00

Fix response attribute error

4326429

proof rule duplicate, I think

3d64512

Rename index in proof response attribute

bfc1a28

Addressing group entry

fa244ff

Trying to fix compilation error

088b8aa

Found compilation error,as response-attribute-arg was accidentally de…

4a069a1

…fined in response cddl

Remove proof prefix

9ae4a15

Fix map association typo

c0d15ed

Appears that a separate entry for attribute-related-index is needed

72bff56

looks like profo prefix is needed

865c9d4

Split proof into proof and proof operation to avoid anonymous enum

fee322c

Misssing memo field?

14fed5e

MavenRain mentioned this pull request Feb 8, 2023

Discover a way to move proof key calculation into background/uniform process liftedinit/many-rs#301

Open

hansl requested changes Feb 22, 2023

View reviewed changes

attributes/network/14_proof.adoc Outdated Show resolved Hide resolved

attributes/network/14_proof.adoc Outdated Show resolved Hide resolved

attributes/network/14_proof.adoc Outdated Show resolved Hide resolved

MavenRain and others added 3 commits February 22, 2023 12:28

Update attributes/network/14_proof.adoc

428dd5b

Co-authored-by: Hans Larsen <hans@larsen.online>

Update attributes/network/14_proof.adoc

01a9054

Co-authored-by: Hans Larsen <hans@larsen.online>

Update attributes/network/14_proof.adoc

051eb9b

Co-authored-by: Hans Larsen <hans@larsen.online>

MavenRain requested a review from hansl February 22, 2023 20:29

hansl approved these changes Feb 22, 2023

View reviewed changes

MavenRain merged commit 9f670fb into many-protocol:main Feb 22, 2023

MavenRain deleted the proof-request-response branch February 22, 2023 20:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add spec for proofs #89

Add spec for proofs #89

MavenRain commented Jan 3, 2023

hansl Jan 3, 2023

MavenRain Jan 3, 2023

hansl Jan 3, 2023 •

edited

Loading

MavenRain Jan 3, 2023

fmorency left a comment

fmorency Jan 3, 2023

fmorency Jan 3, 2023

fmorency Jan 3, 2023

fmorency Jan 3, 2023

hansl Jan 5, 2023

fmorency Jan 3, 2023

hansl Jan 4, 2023

hansl Jan 5, 2023

hansl Jan 5, 2023

hansl Jan 5, 2023

MavenRain Jan 5, 2023

hansl Jan 5, 2023

fmorency left a comment

fmorency Jan 5, 2023

fmorency Jan 5, 2023

MavenRain Jan 5, 2023

fmorency Jan 5, 2023

fmorency Jan 5, 2023

MavenRain Jan 5, 2023

fmorency left a comment

fmorency Jan 10, 2023

fmorency Jan 10, 2023

fmorency Jan 10, 2023

fmorency Jan 10, 2023

hansl Jan 12, 2023

hansl Jan 12, 2023

hansl Jan 12, 2023

MavenRain Jan 13, 2023

	key-value-pair = [3, (0 => bstr, 1 => bstr)]
	key-value-pair = [3, bstr, bstr]


		This is a uint representing the hasher to be used in constructing proofs.

		* 0 - merk default hash (currently blake3)


		This is a uint representing the hasher to be used in constructing proofs.

		* 0 - merk default hash (see xref:../../spec/proof_hash_scheme.adoc.)


		== Attribute Argument

		This is a uint representing the hasher to be used in constructing proofs.

	This is a uint representing the hasher to be used in constructing proofs.
	This is a `uint` representing the hasher used in constructing the proofs.

	* 0 - merk default hash (see xref:../../spec/proof_hash_scheme.adoc.)
	* 0 - `merk` default hash (see xref:../../spec/proof_hash_scheme.adoc.)

		@@ -0,0 +1,17 @@
		; Proof. If requested, this is the set of steps constituting a proof of existence of what was requested in the state of the Merkle tree.
		proof@response-attribute-arg = [ + ( node-hash / key-value-hash / key-value-pair / parent / child ) ]

-proof@response-attribute-arg = [ + ( node-hash / key-value-hash / key-value-pair / parent / child ) ]
+proof@response-attribute-arg = {
+    ; Root application hash.
+=> bstr,
+    ; The proof operations.
+=> proof,
+    ; Extensible attribute related indices for extra information (implementation specific)
+    * attribute-related-index => (),
+}
+proof = [ + ( node-hash / key-value-hash / key-value-pair / parent / child ) ]

	kv_hash = H(0x00, key, value)
	kv_hash = H(0x00, key.length, key, value.length, value)

Add spec for proofs #89

Add spec for proofs #89

Conversation

MavenRain commented Jan 3, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hansl Jan 3, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fmorency 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

fmorency 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

fmorency 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

hansl Jan 3, 2023 •

edited

Loading