Sharding/propose attestations #1219
Conversation
| @@ -129,6 +134,12 @@ private void initDepositContract() { | |||
| contractInit.complete(null); | |||
| } | |||
|
|
|||
| public void initSubscriptions() { | |||
There was a problem hiding this comment.
I think subscribing to specific events should be a part of the component initialization logic that is triggered by WorldManager. Just like ValidatorSearvice.init() works
| block.getAttestations().forEach(at -> beaconAttester.purgeAttestations(at)); | ||
|
|
||
| activeState = activeState | ||
| .withPendingAttestations(mergedAttestations); |
There was a problem hiding this comment.
It's better to have something like addAttestations(List<AttestationRecord> newAttestations) to avoid the above merging.
| List<AttestationRecord> mergedAttestations = new ArrayList<>(); | ||
| mergedAttestations.addAll(activeState.getPendingAttestations()); | ||
| mergedAttestations.addAll(block.getAttestations()); | ||
| block.getAttestations().forEach(at -> beaconAttester.purgeAttestations(at)); |
There was a problem hiding this comment.
What do you think if we drive this part by a pub/sub mechanism? It would let us avoid tight components coupling.
| @@ -67,8 +84,19 @@ public BeaconState applyBlock(Beacon block, BeaconState to) { | |||
| .withDynasty(dynasty) | |||
| .withLastStateRecalc(cycleStartSlot(block)) | |||
| .withFinality(finality); | |||
|
|
|||
| // remove attestations older than last_state_recalc | |||
| List<AttestationRecord> uptodateAttestations = new ArrayList<>(); | |||
There was a problem hiding this comment.
I am starting to think of decoupling crystallized and active state transitions when looking at these lines.
| uptodateAttestations.add(record); | ||
| } | ||
| } | ||
| beaconAttester.removeOldSlots(crystallized.getLastStateRecalc()); |
There was a problem hiding this comment.
The same "pub/sub" note as previous one. I guess it could be even processed by same handler.
| @@ -27,6 +27,7 @@ | |||
| Exist, | |||
| NoParent, | |||
| StateMismatch, | |||
| Invalid, | |||
There was a problem hiding this comment.
It should also be added to ProcessingResult.fromValidation. Looks like we need a note about that or better design to be sure that in future this problem won't arise again
ethereumj-core/src/main/java/org/ethereum/sharding/processing/BeaconChainImpl.java
Outdated
Show resolved
Hide resolved
|
|
||
| Committee[][] committees = crystallized.getDynasty().getCommittees(); | ||
|
|
||
| int index = (int) data.parent.getSlotNumber() % committees.length; |
There was a problem hiding this comment.
Look in ValidatorServiceImpl to see how this logic should look like. Maybe wrap it with some utility method to re-use?
| int index = (int) data.parent.getSlotNumber() % committees.length; | ||
|
|
||
| if (block.getAttestations().isEmpty()) { | ||
| return Invalid; |
There was a problem hiding this comment.
I'd increase the level of verbosity and call it InvalidAttestations
|
|
||
| AttestationRecord proposerAttestation = block.getAttestations().get(0); | ||
|
|
||
| if (proposerAttestation.getSlot() != data.parent.getSlotNumber()) { |
There was a problem hiding this comment.
It, also, should be checked that hash matched, shouldn't it?
This reverts commit 7f483fd
Should resolve #1192, #1193
not finished
need help