Add block and index proofs [ECR-4011] #1355

dmitry-timofeev · 2020-01-10T14:10:22Z

Overview

Added block and index proofs support. They shall be created through Blockchain.

Documented the various proof types: what they prove, what comprises them, how to create them. Added an example for a commonly used proof type.

Also:

Made SignedMessage public to be able to use it in tests; got rid of redundant serialization in #parseFrom.
Added Block#parseFrom and Block#fromMessage.

See: ECR-4011

⚠️ There are some questions marked as TODO (@slowli , @bullet-tooth , @vitvakatu ).

Definition of Done

There are no TODOs left in the code
Change is covered by automated tests
The coding guidelines are followed
Public API has Javadoc
Method preconditions are checked and documented in the Javadoc of the method
Changelog is updated if needed (in case of notable or breaking changes)
The continuous integration build passes

Make SignedMessage public and prevent redundant message serialization in #parseFrom.

Also, remove bad/redundant anchors.

Prevent redundant deserialization.

[skip ci]

dmitry-timofeev · 2020-01-10T14:35:51Z

exonum-java-binding/core/src/main/java/com/exonum/binding/core/blockchain/Blockchain.java

+ *
+ *      Link here with <a href="<relative path>/Blockchain.html#proofs">Blockchain Proofs</a>.
+ *      See also: https://stackoverflow.com/a/27522316/ -->
+ * <h2 id="proofs">Proofs</h2>


⚠️ ECR-4106

The documentation here looks good 👍

[skip ci]

vitvakatu · 2020-01-13T07:52:10Z

exonum-java-binding/core/src/main/java/com/exonum/binding/core/blockchain/Blockchain.java

+     todo: Shall we allow creating proofs for invalid (e.g., impossible) index names or throw
+      an exception?
+
+     todo: If index proofs for "uninitialized" indexes are forbidden, document that.


The native will throw an exception, so I think we should document it.

But

Why doesn't it return a proof of absence (no such index)? Especially as an index is not "initialized" unless (?) something is recorded in it? Doesn't that place extra burden on the service developers to "initialize" an index? Shall they now write an extra "initialize" method in a schema of each service (and we — document that)?

Actually, I don't know. I read sources of proof_for_index method - it looks like explicit check for existence of the index can be removed, and the method will return proof of absence. @slowli am I right?

Yes, if the check of existence is removed, proof_for_index may return proof of absence for a non-existing index. The reason the method is implemented as is currently is that returning proof for a non-existing index doesn't make sense in a typical use case (returning the index proof + proof for data within the index, such as proof for a wallet in the cryptocurrency service).

Thanks! Would you clarify please, if one first requests a proof from an empty index (= no wallets), will it become "initialized"? Will one be able to provide then a proof that the index remains empty?

vitvakatu · 2020-01-13T07:53:33Z

exonum-java-binding/core/src/main/java/com/exonum/binding/core/blockchain/BlockchainProofs.java

+   * @param height the height of the block
+   */
+  static BlockProof createBlockProof(
+      /* todo: here snapshot is not strictly required — but shall we allow Forks (see the ticket) */


Actually I couldn't find any use case for block proofs in execution context (where fork is available), so I propose to keep such restriction for now.

bullet-tooth · 2020-01-13T14:49:04Z

...um-java-binding/core/src/main/java/com/exonum/binding/core/blockchain/proofs/IndexProof.java

+ *
+ * <p>If an index does not exist in the database, then the MapProof will prove its absence.
+ *
+ * @see <a href="../Blockchain.html#service-data-proof">Service Data Proofs</a>


IDEA's quick documentation correctly opens the referenced javadoc, but doesn't recognize anchors. I think it's not the issue for changing links, just fyi

Thanks! That's weird, it works in mine:

Don't anchors work anywhere, even in Blockchain#callerrors?

yeap, Blockchain#callerrors doesn't work as well 🤷‍♀️

bullet-tooth · 2020-01-15T21:43:20Z

...um-java-binding/core/src/main/java/com/exonum/binding/core/blockchain/proofs/BlockProof.java

+ * @see com.exonum.binding.core.blockchain.Block
+ */
+@AutoValue
+public abstract class BlockProof {


what is the reason to have this wrapper? Could we use Proofs.BlockProof directly (and index proof as well)?

The main reason is to have some flexibility to add operations if we release 1.0 without proof verifications. With a wrapper we can add extra operations on top of the proof data if they are needed (e.g., verification).

A side benefit — better documentation, but it can be achieved by other means.

dmitry-timofeev · 2020-01-16T13:24:26Z

We briefly discussed Optional vs Exception in case of uninitialized indexes in createIndexProof with @vitvakatu , and decided to go with an Exception, because eventually we would like to have them all initialized automatically and not be a burden to service developers: ECR-4121.

coveralls · 2020-01-16T14:00:17Z

Coverage decreased (-0.6%) to 85.705% when pulling 6fa70f3 on add-index-proof-ECR-4011 into bfc6cf7 on master.

exonum-java-binding/core/src/main/java/com/exonum/binding/core/service/Schema.java

bullet-tooth · 2020-01-17T08:33:48Z

exonum-java-binding/core/src/main/java/com/exonum/binding/core/service/Schema.java

+ * entire blockchain state and is recorded as such in {@linkplain Block#getStateHash() blocks}
+ * and Precommit messages.
+ *
+ * <p>Exonum starts aggregating a service collection state hash once it is <em>initialized</em>:


Suggested change

* Exonum starts aggregating a service collection state hash once it is initialized:

* Exonum starts aggregating a service collections state hash once it is initialized:

The sentence says when Exonum starts aggregating one (any, a single) collection.

[skip ci]

dmitry-timofeev added 9 commits January 10, 2020 15:36

WIP Add IndexProof

6195976

Make SignedMessage public:

2751fe5

Make SignedMessage public and prevent redundant message serialization in #parseFrom.

Finish tests

1912ce4

Improve documentation on proofs

b23e0cc

Also, remove bad/redundant anchors.

Implement proof constructors

123b951

Todo/fixes

13c4ed5

Fix documentation

c5d3408

Reimplement Block#fromMessage

d8db838

Prevent redundant deserialization.

Fix checkstyle

4260179

[skip ci]

dmitry-timofeev requested review from bullet-tooth and vitvakatu January 10, 2020 14:10

dmitry-timofeev commented Jan 10, 2020

View reviewed changes

dmitry-timofeev added 2 commits January 10, 2020 16:40

Add jira ref

6ed552a

Add changelog entry

eea3ce6

[skip ci]

vitvakatu approved these changes Jan 13, 2020

View reviewed changes

bullet-tooth reviewed Jan 13, 2020

View reviewed changes

bullet-tooth approved these changes Jan 14, 2020

View reviewed changes

bullet-tooth reviewed Jan 16, 2020

View reviewed changes

dmitry-timofeev added 6 commits January 16, 2020 11:19

Document the exception in case no index proof can be created.

253a45a

Update to the new native spec

d47524e

Document the initialization connection

4a1d3ca

Merge remote-tracking branch 'main/master' into add-index-proof-ECR-4011

1fa35e3

Enable the tests depending on the native

0e835cc

Fix checkstyle

d0c47f6

dmitry-timofeev changed the base branch from new-protos-base to master January 16, 2020 11:32

dmitry-timofeev added 3 commits January 16, 2020 13:33

Empty to trigger CI

edd78a4

Fix import

581bf98

Fix native methods names

d85adb2

Merge branch 'master' into add-index-proof-ECR-4011

6fa70f3

dmitry-timofeev requested a review from bullet-tooth January 16, 2020 16:19

bullet-tooth approved these changes Jan 17, 2020

View reviewed changes

s/the core/Exonum/

15df4b7

[skip ci]

dmitry-timofeev merged commit ec6bed2 into master Jan 17, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add block and index proofs [ECR-4011] #1355

Add block and index proofs [ECR-4011] #1355

dmitry-timofeev commented Jan 10, 2020 •

edited

Loading

dmitry-timofeev Jan 10, 2020

slowli Jan 10, 2020

vitvakatu Jan 13, 2020

dmitry-timofeev Jan 13, 2020

vitvakatu Jan 13, 2020

slowli Jan 14, 2020

dmitry-timofeev Jan 15, 2020

vitvakatu Jan 13, 2020

bullet-tooth Jan 13, 2020

dmitry-timofeev Jan 14, 2020

dmitry-timofeev Jan 14, 2020

bullet-tooth Jan 14, 2020

bullet-tooth Jan 15, 2020

dmitry-timofeev Jan 16, 2020

dmitry-timofeev commented Jan 16, 2020

coveralls commented Jan 16, 2020 •

edited

Loading

bullet-tooth Jan 17, 2020

dmitry-timofeev Jan 17, 2020

	* <p>Exonum starts aggregating a service collection state hash once it is <em>initialized</em>:
	* <p>Exonum starts aggregating a service collections state hash once it is <em>initialized</em>:

Add block and index proofs [ECR-4011] #1355

Add block and index proofs [ECR-4011] #1355

Conversation

dmitry-timofeev commented Jan 10, 2020 • edited Loading

Overview

Definition of Done

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

dmitry-timofeev commented Jan 16, 2020

coveralls commented Jan 16, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dmitry-timofeev commented Jan 10, 2020 •

edited

Loading

coveralls commented Jan 16, 2020 •

edited

Loading