Merkle hash chip #98

therealyingtong · 2021-06-02T10:49:18Z

Closes #95.

codecov-commenter · 2021-06-02T11:47:22Z

Codecov Report

Merging #98 (7c38f14) into main (0ccb010) will increase coverage by 31.66%.
The diff coverage is 81.85%.

@@             Coverage Diff             @@
##             main      #98       +/-   ##
===========================================
+ Coverage   52.31%   83.98%   +31.66%     
===========================================
  Files          16       53       +37     
  Lines         583     4657     +4074     
===========================================
+ Hits          305     3911     +3606     
- Misses        278      746      +468

Impacted Files	Coverage Δ
src/constants/commit_ivk_r.rs	`100.00% <ø> (ø)`
src/constants/load.rs	`0.00% <0.00%> (ø)`
src/constants/note_commit_r.rs	`100.00% <ø> (ø)`
src/constants/nullifier_k.rs	`100.00% <ø> (ø)`
src/constants/spend_auth_g.rs	`100.00% <ø> (ø)`
src/constants/value_commit_r.rs	`100.00% <ø> (ø)`
src/primitives/poseidon/grain.rs	`91.04% <ø> (ø)`
src/test_vectors/note_encryption.rs	`100.00% <ø> (ø)`
src/primitives/redpallas.rs	`61.11% <40.00%> (-1.39%)`	⬇️
src/bundle.rs	`42.50% <43.22%> (+42.50%)`	⬆️
... and 88 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 12cef17...7c38f14. Read the comment docs.

src/circuit/gadget/orchard_action/merkle.rs

daira · 2021-06-28T16:50:39Z

src/circuit/gadget/sinsemilla/merkle/chip.rs

+                    copy(
+                        &mut region,
+                        || "copy a",
+                        config.advices[0],


Maybe name these rather than repeating the indices from earlier?

Instead of naming the columns, I have chosen to inline a table in the doc comments showing how they're being used in the custom gate:

The pieces and subpieces are arranged in the following configuration: | A_0 | A_1 | A_2 | A_3 | A_4 | l_plus_1 | ---------------------------------------------------- | a | b | c | left | right | l + 1 | | z1_a | z1_b | b_1 | b_2 | | |

daira · 2021-06-28T17:17:56Z

src/circuit/gadget/sinsemilla/merkle/chip.rs

+            //    z_1 of SinsemillaHash(b) = b_1 + 2^5 b_2
+            // => b_0 = b - (z1_b * 2^10)
+            let z1_b = meta.query_advice(advices[1], Rotation::next());
+            // b_1 has been constrained to be 5 bits outside this gate.


I don't see where this is done (I may have just missed it). Similarly for b_2.

This is done when witness_short_check() is called in hash_layer().

daira

utACK modulo comments and suggestions.

This has three const generic parameters: PATH_LENGTH, K, MAX_WORDS. PATH_LENGTH is the length of the Merkle path being hashed. K and MAX_WORDS parameterize the internal Sinsemilla instance used in hashing the path.

…tions for MerkleChip

MerkleChip::configure() takes a SinsemillaConfig as input.

Co-authored-by: Daira Hopwood <daira@jacaranda.org> Co-authored-by: Jack Grigg <jack@electriccoin.co>

Co-authored-by: Daira Hopwood <daira@jacaranda.org>

src/circuit/gadget/utilities.rs

str4d

utACK 3806a9d with a non-blocking question. Layout LGTM.

src/circuit/gadget/sinsemilla/merkle/chip.rs

str4d · 2021-06-29T19:30:31Z

src/circuit/gadget/sinsemilla/merkle/chip.rs

+        //    is decomposed.
+        //  - Disabling the entire decomposition gate when set to zero
+        //    (i.e. replacing a Selector).
+


Nit: Visually connect the comment to the fixed column:

Suggested change

src/circuit/gadget/sinsemilla/merkle/chip.rs

src/circuit/gadget/utilities.rs

str4d · 2021-06-29T21:45:40Z

I merged my suggestions, after confirming that the lookup_config in MerkleConfig was unused in the action-circuit PR. If it turns out to be necessary later, we can revert that commit.

daira

utACK 7c38f14 (last three commits).

daira · 2021-07-06T08:03:02Z

Hmm, looking at the layout, there's a single cell in the middle column that is taking up its own row in each 5-column side, and there is space in the previous row(s) for it to be moved to.

therealyingtong force-pushed the sinsemilla-chip-config branch from 14b2bbc to f2ed4c8 Compare June 2, 2021 11:25

therealyingtong force-pushed the merkle-chip branch from aca3788 to 9667781 Compare June 2, 2021 11:28

therealyingtong force-pushed the sinsemilla-chip-config branch 3 times, most recently from b279de3 to 5074388 Compare June 3, 2021 04:31

therealyingtong force-pushed the merkle-chip branch 4 times, most recently from b025ffb to 9882fe8 Compare June 3, 2021 18:40

daira reviewed Jun 3, 2021

View reviewed changes

src/circuit/gadget/orchard_action/merkle.rs Outdated Show resolved Hide resolved

daira reviewed Jun 3, 2021

View reviewed changes

src/circuit/gadget/orchard_action/merkle.rs Outdated Show resolved Hide resolved

therealyingtong force-pushed the merkle-chip branch 3 times, most recently from 7669068 to 23e5018 Compare June 4, 2021 12:48

therealyingtong force-pushed the sinsemilla-chip-config branch from 6935a6a to 44dd0eb Compare June 4, 2021 16:33

therealyingtong force-pushed the merkle-chip branch from 23e5018 to ca15012 Compare June 4, 2021 17:01

therealyingtong force-pushed the sinsemilla-chip-config branch from 44dd0eb to edc6ec4 Compare June 5, 2021 00:59

therealyingtong force-pushed the merkle-chip branch from ca15012 to 19bcfa0 Compare June 5, 2021 01:01

therealyingtong mentioned this pull request Jun 5, 2021

Action circuit #101

Merged

therealyingtong force-pushed the sinsemilla-chip-config branch from edc6ec4 to e20c6e6 Compare June 5, 2021 08:30

therealyingtong force-pushed the merkle-chip branch 3 times, most recently from 8a5f541 to 527963c Compare June 5, 2021 09:05

therealyingtong force-pushed the sinsemilla-chip-config branch from e20c6e6 to 96290d9 Compare June 6, 2021 00:10

therealyingtong force-pushed the merkle-chip branch from 527963c to 2731554 Compare June 6, 2021 00:14

therealyingtong force-pushed the sinsemilla-chip-config branch from 96290d9 to 6eba421 Compare June 6, 2021 08:00

therealyingtong force-pushed the merkle-chip branch from 2731554 to 8602455 Compare June 6, 2021 12:13

therealyingtong force-pushed the sinsemilla-chip-config branch from 6eba421 to 5936258 Compare June 6, 2021 12:34

therealyingtong force-pushed the merkle-chip branch from 8602455 to 63ecab5 Compare June 6, 2021 12:36

daira reviewed Jun 28, 2021

View reviewed changes

daira approved these changes Jun 28, 2021

View reviewed changes

therealyingtong and others added 10 commits June 29, 2021 12:07

Add spec::i2lebsp and constants::MERKLE_DEPTH_ORCHARD

1ac3541

sinsemilla::merkle.rs: Add MerkleInstructions.

d090da0

This has three const generic parameters: PATH_LENGTH, K, MAX_WORDS. PATH_LENGTH is the length of the Merkle path being hashed. K and MAX_WORDS parameterize the internal Sinsemilla instance used in hashing the path.

sinsemilla::merkle.rs: Add MerkleChip

68878d8

sinsemilla::merkle.rs: Derive SinsemillaInstructions, CondSwapInstruc…

6976e2b

…tions for MerkleChip

sinsemilla::merkle.rs: Configure MerkleChip

569eb4b

MerkleChip::configure() takes a SinsemillaConfig as input.

sinsemilla::merkle.rs: Implement MerkleInstructions for MerkleChip.

f30de79

Co-authored-by: Daira Hopwood <daira@jacaranda.org> Co-authored-by: Jack Grigg <jack@electriccoin.co>

sinsemilla::merkle.rs: Add test for MerkleChip.

db45c81

Constrain b_1 + 2^5 b_2 = z1_b in decomposition gate.

32e564a

Docfixes, variable renames, cleanups

d68eb65

Co-authored-by: Daira Hopwood <daira@jacaranda.org>

Further cleanups and docfixes.

3806a9d

Co-authored-by: Daira Hopwood <daira@jacaranda.org>

therealyingtong force-pushed the merkle-chip branch from c42b99c to 3806a9d Compare June 29, 2021 04:11

therealyingtong mentioned this pull request Jun 29, 2021

Introduce LookupRangeCheckConfigs for each Sinsemilla advice column #133

Merged

therealyingtong changed the base branch from patch-sinsemilla to main June 29, 2021 09:40

therealyingtong changed the base branch from main to patch-sinsemilla June 29, 2021 09:41

Base automatically changed from patch-sinsemilla to main June 29, 2021 09:43

str4d reviewed Jun 29, 2021

View reviewed changes

src/circuit/gadget/utilities.rs Outdated Show resolved Hide resolved

str4d approved these changes Jun 29, 2021

View reviewed changes

str4d reviewed Jun 29, 2021

View reviewed changes

src/circuit/gadget/sinsemilla/merkle/chip.rs Outdated Show resolved Hide resolved

Remove unused lookup_config in MerkleConfig

8dfcd7d

str4d reviewed Jun 29, 2021

View reviewed changes

src/circuit/gadget/utilities.rs Outdated Show resolved Hide resolved

Optimize transpose_option_array

cbded2b

rustfmt

7c38f14

daira approved these changes Jun 29, 2021

View reviewed changes

str4d merged commit bb159a2 into main Jun 29, 2021

str4d deleted the merkle-chip branch June 29, 2021 22:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Merkle hash chip #98

Merkle hash chip #98

therealyingtong commented Jun 2, 2021 •

edited

Loading

codecov-commenter commented Jun 2, 2021 •

edited

Loading

daira Jun 28, 2021

therealyingtong Jun 29, 2021

daira Jun 28, 2021

therealyingtong Jun 29, 2021

daira left a comment

str4d left a comment

str4d Jun 29, 2021

str4d commented Jun 29, 2021

daira left a comment

daira commented Jul 6, 2021

Merkle hash chip #98

Merkle hash chip #98

Conversation

therealyingtong commented Jun 2, 2021 • edited Loading

codecov-commenter commented Jun 2, 2021 • edited Loading

Codecov Report

daira Jun 28, 2021

Choose a reason for hiding this comment

therealyingtong Jun 29, 2021

Choose a reason for hiding this comment

daira Jun 28, 2021

Choose a reason for hiding this comment

therealyingtong Jun 29, 2021

Choose a reason for hiding this comment

daira left a comment

Choose a reason for hiding this comment

str4d left a comment

Choose a reason for hiding this comment

str4d Jun 29, 2021

Choose a reason for hiding this comment

str4d commented Jun 29, 2021

daira left a comment

Choose a reason for hiding this comment

daira commented Jul 6, 2021

therealyingtong commented Jun 2, 2021 •

edited

Loading

codecov-commenter commented Jun 2, 2021 •

edited

Loading