Add proof type #245

MavenRain · 2023-01-19T18:44:37Z

The proof type will be used to construct the response attribute arguments for proofs in many-framework where proofs are requested (via attribute 3).

Notes:

I hit snags trying to add context parameters to the idstore and event endpoints (weird compiler errors complaining about not implementing the Decode trait).
events.list will require implementing construction of proofs from key ranges.
Key retrieval for proofs is still bespoke by endpoint. It would be nice to find a way of pushing key collection to the background.
Migration work related to hash scheme is out of scope for this PR.

MavenRain · 2023-01-19T19:39:14Z

src/many-migration/src/lib.rs

@@ -554,7 +554,6 @@ impl<'a, T, E, IDX: AsRef<str>> Index<IDX> for MigrationSet<'a, T, E> {
 }

 /// Kept for backward compatibility.
-#[deprecated = "Should use MigrationSet::load() instead."]


These warnings were triggering on builds. Either these should be removed, or they should be re-added along with the remedies for the places where they are currently being used.

codecov-commenter · 2023-01-24T22:17:24Z

Codecov Report

Merging #245 (09842be) into main (37e273b) will decrease coverage by 0.16%.
The diff coverage is 75.35%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##             main     #245      +/-   ##
==========================================
- Coverage   73.33%   73.17%   -0.16%     
==========================================
  Files         175      177       +2     
  Lines       22715    23171     +456     
==========================================
+ Hits        16657    16956     +299     
- Misses       6058     6215     +157

Impacted Files	Coverage Δ
src/many-error/src/error.rs	`46.24% <ø> (ø)`
src/many-ledger/src/module/ledger_mintburn.rs	`0.00% <0.00%> (ø)`
src/many-ledger/src/storage/ledger_mintburn.rs	`0.00% <0.00%> (ø)`
src/many-macros/src/lib.rs	`0.21% <0.00%> (-0.03%)`	⬇️
src/many-modules/src/_5_data.rs	`92.52% <0.00%> (-0.88%)`	⬇️
src/many-protocol/src/lib.rs	`96.80% <ø> (ø)`
src/many-protocol/src/response.rs	`88.46% <0.00%> (-1.99%)`	⬇️
src/many-types/src/attributes.rs	`88.20% <0.00%> (+0.20%)`	⬆️
src/many-types/src/lib.rs	`75.06% <ø> (ø)`
src/many/src/main.rs	`0.24% <0.00%> (-0.02%)`	⬇️
... and 49 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

hansl

First round of reviews.

Can you also add an issue where we have proper Proof tree algebra and can just pass the context around and collect keys as we go instead of returning them everywhere? Should simplify the code.

src/many/src/main.rs

hansl · 2023-02-16T20:48:51Z

src/many-protocol/src/context.rs

+    }
+}
+
+impl From<Context> for Cow<'_, Context> {


Cow should support Into or From already, no? Also, why is this necessary?

@hansl I'm going to remove this because I thought I would need this flexibility at the beginning of writing this PR but then quickly discovered that I only needed to ever move a single Context object between calls. For the record, though, I don't believe that Cow comes with automatic From/Into support except for a few specific cases. For user-defined types, one must define From for Cow support.

src/many-kvstore/Cargo.toml

src/many-ledger/src/storage/ledger.rs

hansl · 2023-02-16T22:09:32Z

src/many-ledger/src/storage/ledger.rs

+                        })
+                        .collect::<Result<Vec<_>, _>>()
+                })
+                .map_err(|error| ManyError::unknown(error.to_string()))


Suggested change

.map_err(|error| ManyError::unknown(error.to_string()))

.map_err(ManyError::unknown)

src/many-ledger/src/storage/ledger.rs

hansl · 2023-02-16T22:26:58Z

src/many-ledger/test-utils/src/lib.rs

-        )?;
-        Ok(())
+        self.module_impl
+            .send(


Proofs on a command (like send or mint) are senseless as they are executed in order and the root hash is being updated as we go through the block. So you can't actually use the proof for anything, really.

We should just return proofs on queries, wdyt?

I think this makes sense. I can remove proof functionality from send and mint.

hansl · 2023-02-16T22:28:16Z

src/many-kvstore/src/module/account.rs

+        self.storage
+            .prove_state(context, keys)
+            .map(|error| ManyError::unknown(error.to_string()))
+            .map(Err)
+            .unwrap_or(Ok(()))
+            .map(|_| account::CreateReturn { id })


This should simplify with the other changes to prove_state below to:

Suggested change

self.storage

.prove_state(context, keys)

.map(|error| ManyError::unknown(error.to_string()))

.map(Err)

.unwrap_or(Ok(()))

.map(|_| account::CreateReturn { id })

self.storage.prove_state(context, keys)?;

Ok(account::CreateReturn { id })

Here and everywhere.

src/many-types/src/proof.rs

hansl · 2023-02-16T22:31:04Z

src/many-modules/src/_9_account.rs

@@ -464,50 +465,55 @@ pub type AddFeaturesReturn = EmptyReturn;
 #[cfg_attr(test, mockall::automock)]
 pub trait AccountModuleBackend: Send {
    /// Create an account.
-    fn create(&mut self, sender: &Address, args: CreateArgs) -> Result<CreateReturn, ManyError>;
+    fn create(&mut self, sender: &Address, args: CreateArgs, context: Context) -> Result<CreateReturn, ManyError>;


Commands (that have mut self) don't need to return a proof and cannot actually build a usable one.

hansl · 2023-02-16T23:21:30Z

src/many-protocol/src/context.rs

+    }
+}
+
+impl From<(RequestMessage, Sender<ProofResult>)> for Context {


Additional; Can you make a proper new method instead of Into?

MavenRain · 2023-02-17T17:53:07Z

First round of reviews.

Can you also add an issue where we have proper Proof tree algebra and can just pass the context around and collect keys as we go instead of returning them everywhere? Should simplify the code.

#310

… of prove method of context

hansl · 2023-02-21T23:42:51Z

src/many/src/main.rs

@@ -200,7 +200,7 @@ struct MessageOpt {
    data: Option<String>,

    #[clap(long)]
-    attribute: Option<u32>,
+    proof: Option<bool>,


Doesn't have to be Option<bool>, just set default value.

Sorry, I can make a suggestion :) I was going quick through the PR

Also noted that there's a missing description for this flag.

/// Request a proof of the value. This may cause an error if the server /// does not support proofs, and might not work on all endpoints. Consult /// the specification for more information. #[clap(long, default_value = "false")] proof: bool,

Ah . . . I hadn't done it this way, but will now

hansl

Assuming the proof flag is properly updated, LGTM.

Add proof type

1853d5a

MavenRain requested review from hansl and fmorency January 19, 2023 18:44

MavenRain added 2 commits January 19, 2023 11:08

Remove deprecation warnings

a7f129e

Clippy error

587d080

MavenRain commented Jan 19, 2023

View reviewed changes

MavenRain added 7 commits January 23, 2023 08:24

Rename Proof to ProofOperation

b936af9

Introduce context type

ef86b26

Introduce concept of Context to many-module macros

d0b6455

Want to be able to add context to the balance call but can't

98415a8

Fix macro for context expansion

e0f07ad

Rectified merge with main branch

27e611f

Only look for proof transmission if requested

cc04eef

MavenRain added 8 commits January 25, 2023 10:05

Add convenience methods to Context

aef8b1f

Add context to mint and burn functions

4996045

Merge https://github.com/liftedinit/many-rs into oobi/proof

bdc5cf9

Move toward collecting proof from context in ProofOperation form

b4c9b15

Move toward collecting proof from context in ProofOperation form

7a8a46a

Relax iterator requirement to something implementing IntoIterator

a15e9f9

Now working with fallible attribute list after CborAny decoding

576f50d

Remove unneeded method

493e43f

MavenRain mentioned this pull request Jan 26, 2023

As an API client, I want to verify that the response from a server is verifiable proof liftedinit/roadmap#60

Closed

MavenRain added 7 commits January 26, 2023 15:01

Post monorepo merge of many-framework portion of PR

cab84be

Merge https://github.com/liftedinit/many-rs into oobi/proof

248b1bd

Proving store concept

e9e9ae0

Proving store changes moved to separate branch

15d24ba

Refactor get_multiple_balances

16a5577

Add ability to supply request attribute

1a41d0c

Print AttributeSet from the response when many message is sent

c55748f

Rectified merge with main branch

e2873c4

hansl suggested changes Feb 16, 2023

View reviewed changes

hansl reviewed Feb 16, 2023

View reviewed changes

MavenRain added 8 commits February 20, 2023 23:09

Refactor attribute to proof parameter in CLI and refactor return type…

ad9c5b8

… of prove method of context

Remove proof functionality from send command

5958211

Remove proof functionality from mint/burn

5163736

TryFrom for Proof to CborAny

c983f14

Remove proof support from ledger token command calls

4c987de

Remove proof functionality from account commands

50aca99

Constructor for Context made

fecb5a9

Merge https://github.com/liftedinit/many-rs into oobi/proof

09842be

MavenRain requested a review from hansl February 21, 2023 20:41

hansl reviewed Feb 21, 2023

View reviewed changes

hansl approved these changes Feb 22, 2023

View reviewed changes

MavenRain added 13 commits February 21, 2023 16:33

Default proof command line value

692bba2

Overcome clippy error

a11eb34

Use default clap option for proof boolean

18fc554

Try to fix bats error, attempt 1

7e5d801

Refactoring proof argument calculation in main

373c55b

Pattern-match option for proof argument calculation

8dc5d19

Prefer collect to from_iter

a3f668f

Prefer collect to from_iter, second change

bff164c

Prefer collect to from_iter, third change

0832ad4

Merge https://github.com/liftedinit/many-rs into oobi/proof

c4e575b

Fix lock files

230050d

Some bazel changes

411b8ad

Format changes made

728cb06

MavenRain merged commit 3c641d8 into liftedinit:main Feb 22, 2023

MavenRain deleted the oobi/proof branch February 22, 2023 19:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add proof type #245

Add proof type #245

MavenRain commented Jan 19, 2023 •

edited

Loading

MavenRain Jan 19, 2023

codecov-commenter commented Jan 24, 2023 •

edited

Loading

hansl left a comment

hansl Feb 16, 2023

MavenRain Feb 21, 2023

hansl Feb 16, 2023

hansl Feb 16, 2023

MavenRain Feb 21, 2023

hansl Feb 16, 2023

hansl Feb 16, 2023

hansl Feb 16, 2023

MavenRain commented Feb 17, 2023

hansl Feb 21, 2023

MavenRain Feb 22, 2023

hansl Feb 22, 2023

hansl Feb 22, 2023

MavenRain Feb 22, 2023

MavenRain Feb 22, 2023

hansl left a comment

	.map_err(\|error\| ManyError::unknown(error.to_string()))
	.map_err(ManyError::unknown)

Add proof type #245

Add proof type #245

Conversation

MavenRain commented Jan 19, 2023 • edited Loading

Choose a reason for hiding this comment

codecov-commenter commented Jan 24, 2023 • edited Loading

Codecov Report

hansl 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

MavenRain commented Feb 17, 2023

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 left a comment

Choose a reason for hiding this comment

MavenRain commented Jan 19, 2023 •

edited

Loading

codecov-commenter commented Jan 24, 2023 •

edited

Loading