diff --git a/chain/src/main/java/org/ethereum/beacon/chain/DefaultBeaconChain.java b/chain/src/main/java/org/ethereum/beacon/chain/DefaultBeaconChain.java index a7cc6c1bf..3763c1c9d 100644 --- a/chain/src/main/java/org/ethereum/beacon/chain/DefaultBeaconChain.java +++ b/chain/src/main/java/org/ethereum/beacon/chain/DefaultBeaconChain.java @@ -126,7 +126,7 @@ public synchronized ImportResult insert(BeaconBlock block) { BeaconTuple newTuple = BeaconTuple.of(block, postBlockState); tupleStorage.put(newTuple); - updateFinality(parentState, postBlockState); + updateFinality(postBlockState); chainStorage.commit(); @@ -153,21 +153,32 @@ public BeaconTuple getRecentlyProcessed() { return recentlyProcessed; } - private void updateFinality(BeaconState previous, BeaconState current) { - if (!previous.getFinalizedCheckpoint().equals(current.getFinalizedCheckpoint())) { + private void updateFinality(BeaconState current) { + boolean finalizedStorageUpdated = false; + boolean justifiedStorageUpdated = false; + if (current + .getFinalizedCheckpoint() + .getEpoch() + .greater(fetchFinalizedCheckpoint().getEpoch())) { chainStorage.getFinalizedStorage().set(current.getFinalizedCheckpoint()); + finalizedStorageUpdated = true; + } + if (current + .getCurrentJustifiedCheckpoint() + .getEpoch() + .greater(fetchJustifiedCheckpoint().getEpoch())) { + chainStorage.getJustifiedStorage().set(current.getCurrentJustifiedCheckpoint()); + justifiedStorageUpdated = true; + } + // publish updates after both storages have been updated + // the order can be important if a finalizedCheckpointStream subscriber will look + // into justified storage + // in general, it may be important to publish after commit has succeeded + if (finalizedStorageUpdated) { finalizedCheckpointStream.onNext(current.getFinalizedCheckpoint()); } - if (!previous.getCurrentJustifiedCheckpoint().equals(current.getCurrentJustifiedCheckpoint())) { - // store new justified checkpoint if its epoch greater than previous one - if (current - .getCurrentJustifiedCheckpoint() - .getEpoch() - .greater(fetchJustifiedCheckpoint().getEpoch())) { - chainStorage.getJustifiedStorage().set(current.getCurrentJustifiedCheckpoint()); - } - - justifiedCheckpointStream.onNext(current.getFinalizedCheckpoint()); + if (justifiedStorageUpdated) { + justifiedCheckpointStream.onNext(current.getCurrentJustifiedCheckpoint()); } } diff --git a/consensus/src/main/java/org/ethereum/beacon/consensus/spec/EpochProcessing.java b/consensus/src/main/java/org/ethereum/beacon/consensus/spec/EpochProcessing.java index ac816272b..e007069c1 100644 --- a/consensus/src/main/java/org/ethereum/beacon/consensus/spec/EpochProcessing.java +++ b/consensus/src/main/java/org/ethereum/beacon/consensus/spec/EpochProcessing.java @@ -131,7 +131,7 @@ default Pair> get_winning_crosslink_and_attestin Gwei b2 = get_attesting_balance(state, attestations.stream().filter(a -> a.getData().getCrosslink().equals(c2)).collect(toList())); if (b1.equals(b2)) { - return c1.getDataRoot().toString().compareTo(c2.getDataRoot().toString()); + return c1.getDataRoot().compareTo(c2.getDataRoot()); } else { return b1.compareTo(b2); } diff --git a/consensus/src/main/java/org/ethereum/beacon/consensus/spec/ForkChoice.java b/consensus/src/main/java/org/ethereum/beacon/consensus/spec/ForkChoice.java index 4afb89087..bc3853bb8 100644 --- a/consensus/src/main/java/org/ethereum/beacon/consensus/spec/ForkChoice.java +++ b/consensus/src/main/java/org/ethereum/beacon/consensus/spec/ForkChoice.java @@ -10,6 +10,7 @@ import org.ethereum.beacon.core.types.Gwei; import org.ethereum.beacon.core.types.SlotNumber; import org.ethereum.beacon.core.types.ValidatorIndex; +import org.javatuples.Pair; import tech.pegasys.artemis.ethereum.core.Hash32; /** @@ -109,7 +110,7 @@ default Hash32 get_head(Store store) { } head = children.stream() - .max(Comparator.comparing(root -> get_latest_attesting_balance(store, root))) + .max(Comparator.comparing(root -> Pair.with(get_latest_attesting_balance(store, root), root))) .get(); } } diff --git a/types/src/test/java/tech/pegasys/artemis/util/bytes/BytesValueTest.java b/types/src/test/java/tech/pegasys/artemis/util/bytes/BytesValueTest.java index 762682236..27abc3fc0 100644 --- a/types/src/test/java/tech/pegasys/artemis/util/bytes/BytesValueTest.java +++ b/types/src/test/java/tech/pegasys/artemis/util/bytes/BytesValueTest.java @@ -16,7 +16,6 @@ import io.netty.buffer.ByteBuf; import io.netty.buffer.Unpooled; import io.vertx.core.buffer.Buffer; -import net.consensys.cava.bytes.MutableBytes; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; @@ -857,6 +856,49 @@ public void testBytesValuesComparatorReturnsMatchUnsignedValueByteValue() { assertThat(small.compareTo(otherSmall)).isEqualTo(0); } + @Test + public void testBytesValueComparator_lexicographical_order() { + checkOrder(h("0x00"), h("0x01")); + checkOrder(h("0x01"), h("0x7f")); + checkOrder(h("0x7f"), h("0x80")); + checkOrder(h("0x80"), h("0xff")); + + checkOrder(h("0x0000"), h("0x0001")); + checkOrder(h("0x0001"), h("0x007f")); + checkOrder(h("0x007f"), h("0x0080")); + checkOrder(h("0x0080"), h("0x00ff")); + + checkOrder(h("0x0000"), h("0x0100")); + checkOrder(h("0x0100"), h("0x7f00")); + checkOrder(h("0x7f00"), h("0x8000")); + checkOrder(h("0x8000"), h("0xff00")); + + checkOrder(h("0x00"), h("0x0100")); + checkOrder(h("0x01"), h("0x7f00")); + checkOrder(h("0x7f"), h("0x8000")); + checkOrder(h("0x80"), h("0xff00")); + + checkOrder(h("0x00"), h("0x01ff")); + checkOrder(h("0x01"), h("0x7fff")); + checkOrder(h("0x7f"), h("0x80ff")); + checkOrder(h("0x80"), h("0xffff")); + + checkOrder(h("0x0001"), h("0x0100")); + checkOrder(h("0x007f"), h("0x7f00")); + checkOrder(h("0x0080"), h("0x8000")); + checkOrder(h("0x00ff"), h("0xff00")); + + checkOrder(h("0x000001"), h("0x010000")); + checkOrder(h("0x00007f"), h("0x7f0000")); + checkOrder(h("0x000080"), h("0x800000")); + checkOrder(h("0x0000ff"), h("0xff0000")); + } + + static void checkOrder(BytesValue lesser, BytesValue greater) { + assertThat(lesser).isLessThan(greater); + assertThat(greater).isGreaterThan(lesser); + } + @Test public void testGetSetBit() { MutableBytesValue bytes = MutableBytesValue.create(4);