Write Enrollments data to the block. #504

TrustHenry · 2020-01-30T05:14:54Z

Writing Enrollment data to a block determines the height of the block that can participate in the validation as a validator.
Validators needed for the consensus process can be determined and the number of validators between nodes can be set.

The Enrollment data should be recorded only in the enroll start block.
The node store the enrolled block height.

Reach consensus on transaction and enrollment set
The enrollments to be included in a block must first
be voted on by the consensus protocol.

Relates to #493 Enrollment data should be written to the block.
~~Relates to #565 Make Enrollment Part of Consensus~~

TrustHenry · 2020-01-30T05:16:32Z

~~Writing unit test code on ledger.~~
Ready

codecov · 2020-01-30T05:22:25Z

Codecov Report

Merging #504 into v0.x.x will decrease coverage by 0.88%.
The diff coverage is 99.15%.

@@            Coverage Diff             @@
##           v0.x.x     #504      +/-   ##
==========================================
- Coverage   89.78%   88.89%   -0.89%     
==========================================
  Files          62       61       -1     
  Lines        4463     4556      +93     
==========================================
+ Hits         4007     4050      +43     
- Misses        456      506      +50

Flag	Coverage Δ
#integration	`?`
#unittests	`88.89% <99.15%> (+0.27%)`	⬆️

Impacted Files	Coverage Δ
tests/unit/BlockStorageMultiTx.d	`100% <100%> (ø)`	⬆️
source/agora/consensus/Validation.d	`98.87% <100%> (-0.01%)`	⬇️
tests/unit/BlockStorage.d	`100% <100%> (ø)`	⬆️
source/agora/consensus/data/Block.d	`100% <100%> (+0.7%)`	⬆️
tests/unit/BlockStorageChecksum.d	`100% <100%> (ø)`	⬆️
source/agora/node/Ledger.d	`96.73% <99.1%> (+0.64%)`	⬆️
source/agora/common/Task.d	`0% <0%> (-100%)`	⬇️
source/agora/node/Node.d	`77.38% <0%> (-21.45%)`	⬇️
source/agora/common/Config.d	`74.64% <0%> (-16.2%)`	⬇️
source/agora/network/NetworkManager.d	`69.86% <0%> (-4.8%)`	⬇️
... and 4 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 40dd4a1...577687c. Read the comment docs.

source/agora/consensus/data/Block.d

AndrejMitrovic · 2020-01-31T01:30:24Z

I'm not sure if the way the commits were split makes sense. All of these seem like they atomically belong together as a single commit.

@Geod24 do you agree?

TrustHenry · 2020-01-31T01:51:30Z

I'm not sure if the way the commits were split makes sense. All of these seem like they atomically belong together as a single commit.

If this looks like it, I'll make these committs single-commit

source/agora/consensus/data/Block.d

AndrejMitrovic · 2020-01-31T05:24:20Z

source/agora/node/Ledger.d

@@ -68,17 +73,21 @@ public class Ledger
            pool = the transaction pool
            utxo_set = the set of unspent outputs
            storage = the block storage
+            enroll_man = the enrollmentManager
+            node_config = the node config


Nice catch. 👍

source/agora/node/Ledger.d

AndrejMitrovic · 2020-01-31T05:30:31Z

source/agora/node/Ledger.d

@@ -236,6 +249,11 @@ public class Ledger
        if (!this.storage.saveBlock(block))
            assert(0);

+        foreach (enrollment; block.header.enrollments)
+            if (!this.enroll_man.updateEnrolledHeight(enrollment.utxo_key,


This is a great example of what @Geod24 talked about. If updateEnrolledHeight fails, we shut down the node immediately with assert(0);.

But then the question is, why is updateEnrolledHeight returning a boolean status code? It should either throw an Error or just assert(0) itself, and the function should be made nothrow and not return any status code.

I will file a separate issue for this, so you don't have to change anything here @TrustHenry.

I was thinking the same thing.

source/agora/node/Ledger.d

TrustHenry · 2020-02-03T07:59:44Z

Ready for review.

TrustHenry · 2020-02-06T05:22:02Z

@Geod24 @AndrejMitrovic ping.

TrustHenry · 2020-02-06T06:59:16Z

I rebased it.

Geod24 · 2020-03-03T11:04:59Z

You can also probably isolate Defines the data used when reaching consensus in its own commit.
The tests are not great though: the init values are all 0, so they don't test much.
For the transaction set, try to use the genesis txs (see Set.from) and for the Enrollment, just add twice this entry

Geod24 · 2020-03-03T11:25:03Z

Needs a rebase too ;)

Geod24 · 2020-03-04T00:40:36Z

This needs a rebase. And perhaps Reach consensus on transaction and enrollment set can be split into its own PR.

Geod24 · 2020-03-04T02:20:03Z

Looks like you removed the consensus commit ?

TrustHenry · 2020-03-04T02:20:34Z

Except for the part that was an issue here, only the part that registered data was registered on the block was left.
Related issues are #380 , #565
I will make another PR for the Enrollment Consensus part.

TrustHenry · 2020-03-04T02:22:42Z

Ready to review.

Geod24 · 2020-03-04T03:44:33Z

node-2_1_6744a39c3d6b | 2020-03-04T03:15:17.120102785Z 2020-03-04 03:15:16,842 Error [agora.consensus.protocol.Nominator] - validateValue(): Invalid tx set: Transaction: Input ref not in UTXO

Geod24 · 2020-03-04T06:56:04Z

I will make another PR for the Enrollment Consensus part.

Could you do it now ? That commit was ready, and it's much better to make consensus before writing to the block :)

TrustHenry · 2020-03-04T09:15:30Z

I will make another PR for the Enrollment Consensus part.

Could you do it now ? That commit was ready, and it's much better to make consensus before writing to the block :)

I made PR #620

TrustHenry · 2020-03-05T06:24:17Z

I am doing solving this conflict.

Geod24 · 2020-03-05T06:56:40Z

Ping me when you have rebased, let's get this merged!

Add the elements into the block. The enrollments update enrolledHeight Enrollment data should be written to the block For unnittest

TrustHenry · 2020-03-05T07:39:48Z

@Geod24 ping

TrustHenry requested review from AndrejMitrovic and Geod24 January 30, 2020 05:16

TrustHenry self-assigned this Jan 30, 2020

TrustHenry added the type-feature An addition to the system introducing new functionalities label Jan 30, 2020

TrustHenry mentioned this pull request Jan 30, 2020

Add logic to validate enrollments of blockheader #480

Merged

AndrejMitrovic reviewed Jan 31, 2020

View reviewed changes

source/agora/consensus/data/Block.d Outdated Show resolved Hide resolved

TrustHenry force-pushed the writeEnrollment branch 2 times, most recently from f16eaf7 to 1164b38 Compare January 31, 2020 02:06

AndrejMitrovic suggested changes Jan 31, 2020

View reviewed changes

AndrejMitrovic mentioned this pull request Jan 31, 2020

Replace non-recoverable bool return code methods with nothrow 'assert' methods #514

Closed

TrustHenry force-pushed the writeEnrollment branch 5 times, most recently from 7b75d86 to 6384328 Compare February 3, 2020 07:58

TrustHenry changed the title ~~[WIP] Enrollments data write to the block.~~ Enrollments data write to the block. Feb 3, 2020

TrustHenry force-pushed the writeEnrollment branch from 6384328 to 8c854a7 Compare February 3, 2020 09:24

TrustHenry requested a review from AndrejMitrovic February 4, 2020 01:01

TrustHenry force-pushed the writeEnrollment branch 4 times, most recently from e3fb1d0 to 9f17b2e Compare February 6, 2020 05:19

TrustHenry force-pushed the writeEnrollment branch from 9f17b2e to f326bc8 Compare February 6, 2020 06:58

TrustHenry mentioned this pull request Mar 3, 2020

Refactor system integration tests slightly #615

Merged

TrustHenry force-pushed the writeEnrollment branch from d07272d to a67d75b Compare March 3, 2020 11:04

TrustHenry force-pushed the writeEnrollment branch 3 times, most recently from fd2d4e3 to bf0d1eb Compare March 3, 2020 12:08

TrustHenry mentioned this pull request Mar 4, 2020

Defines the data used when reaching consensus #616

Merged

Geod24 mentioned this pull request Mar 4, 2020

Make Set use an RBTree, or a sorted data structure #380

Open

TrustHenry force-pushed the writeEnrollment branch 2 times, most recently from 9cd3d74 to ff3c2b8 Compare March 4, 2020 02:14

TrustHenry force-pushed the writeEnrollment branch 2 times, most recently from 7a6c9ef to 15fa382 Compare March 4, 2020 06:36

Write Enrollment data to the block.

577687c

Add the elements into the block. The enrollments update enrolledHeight Enrollment data should be written to the block For unnittest

TrustHenry force-pushed the writeEnrollment branch from 15fa382 to 577687c Compare March 5, 2020 07:30

Geod24 approved these changes Mar 5, 2020

View reviewed changes

MichaelKim20 self-requested a review March 5, 2020 08:51

Geod24 merged commit 7492b80 into bosagora:v0.x.x Mar 5, 2020

Geod24 mentioned this pull request Mar 5, 2020

Enrollment data should be written to the block. #493

Closed

3 tasks

TrustHenry deleted the writeEnrollment branch May 19, 2020 01:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Write Enrollments data to the block. #504

Write Enrollments data to the block. #504

TrustHenry commented Jan 30, 2020 •

edited

Loading

TrustHenry commented Jan 30, 2020 •

edited

Loading

codecov bot commented Jan 30, 2020 •

edited

Loading

AndrejMitrovic commented Jan 31, 2020

TrustHenry commented Jan 31, 2020 •

edited

Loading

AndrejMitrovic Jan 31, 2020

AndrejMitrovic Jan 31, 2020

TrustHenry Jan 31, 2020

TrustHenry commented Feb 3, 2020

TrustHenry commented Feb 6, 2020

TrustHenry commented Feb 6, 2020

Geod24 commented Mar 3, 2020

Geod24 commented Mar 3, 2020

Geod24 commented Mar 4, 2020

Geod24 commented Mar 4, 2020

TrustHenry commented Mar 4, 2020

TrustHenry commented Mar 4, 2020

Geod24 commented Mar 4, 2020

Geod24 commented Mar 4, 2020

TrustHenry commented Mar 4, 2020

TrustHenry commented Mar 5, 2020

Geod24 commented Mar 5, 2020

TrustHenry commented Mar 5, 2020

Write Enrollments data to the block. #504

Write Enrollments data to the block. #504

Conversation

TrustHenry commented Jan 30, 2020 • edited Loading

TrustHenry commented Jan 30, 2020 • edited Loading

codecov bot commented Jan 30, 2020 • edited Loading

Codecov Report

AndrejMitrovic commented Jan 31, 2020

TrustHenry commented Jan 31, 2020 • edited Loading

AndrejMitrovic Jan 31, 2020

Choose a reason for hiding this comment

AndrejMitrovic Jan 31, 2020

Choose a reason for hiding this comment

TrustHenry Jan 31, 2020

Choose a reason for hiding this comment

TrustHenry commented Feb 3, 2020

TrustHenry commented Feb 6, 2020

TrustHenry commented Feb 6, 2020

Geod24 commented Mar 3, 2020

Geod24 commented Mar 3, 2020

Geod24 commented Mar 4, 2020

Geod24 commented Mar 4, 2020

TrustHenry commented Mar 4, 2020

TrustHenry commented Mar 4, 2020

Geod24 commented Mar 4, 2020

Geod24 commented Mar 4, 2020

TrustHenry commented Mar 4, 2020

TrustHenry commented Mar 5, 2020

Geod24 commented Mar 5, 2020

TrustHenry commented Mar 5, 2020

TrustHenry commented Jan 30, 2020 •

edited

Loading

TrustHenry commented Jan 30, 2020 •

edited

Loading

codecov bot commented Jan 30, 2020 •

edited

Loading

TrustHenry commented Jan 31, 2020 •

edited

Loading