Skip to content

Commit

Permalink
Use Instant to replace RichInstant usage whenever possible (#1462)
Browse files Browse the repository at this point in the history 
* Replace RichInstant with Instant

in MerkleNetworkContext, hence reduce the conversion
between these two types.

* Add MerkleNetworkContext.deserialize() null-safety and avoid transient ISS (#1464)

* Add null-safety and avoid transient ISS from MerkleNetworkContext fast-copy sharing FeeMultiplierSource congestionLevelStarts

Signed-off-by: tinker-michaelj <[email protected]>

* Fix unit test

Signed-off-by: tinker-michaelj <[email protected]>

* 01399 m yacli valide ip and port address book update (#1454)

* add checks to validate port number in service endpoints when uploading an addressBook using yahcli

Signed-off-by: anighanta <[email protected]>

* add test addressbook for bad port in endpoint

Signed-off-by: anighanta <[email protected]>

* updated gitignore to not track script files from yahcli/assests

Signed-off-by: anighanta <[email protected]>

* screened launch script

Signed-off-by: anighanta <[email protected]>

* Resolve SonarCloud Minor Bugs (#1449)

* fix few minor sonarcloud bugs

Signed-off-by: Neeharika-Sompalli <[email protected]>

* Additional fixes

Signed-off-by: Neeharika-Sompalli <[email protected]>

* fix code smell

Signed-off-by: Neeharika-Sompalli <[email protected]>

* move duplicated code to a method

Signed-off-by: Neeharika-Sompalli <[email protected]>

* fix other minor bugs

Signed-off-by: Neeharika-Sompalli <[email protected]>

* fix other issues

Signed-off-by: Neeharika-Sompalli <[email protected]>

* remove unnecessary toString() calls

Signed-off-by: Neeharika-Sompalli <[email protected]>

* add braces

Signed-off-by: Neeharika-Sompalli <[email protected]>

* remove a few casts

Signed-off-by: Neeharika-Sompalli <[email protected]>

* convert getCommonTransactionBodyBytes and legacyReceiptStorageSecs to return long and remove casts

Signed-off-by: Neeharika-Sompalli <[email protected]>

* fix other bugs created

Signed-off-by: Neeharika-Sompalli <[email protected]>

* Address review comments and fix alignment in SmartContractFeeBuilder

Signed-off-by: Neeharika-Sompalli <[email protected]>

* address few review comments

Signed-off-by: Neeharika-Sompalli <[email protected]>

* fix few other review points

Signed-off-by: Neeharika-Sompalli <[email protected]>

* fix typo

Signed-off-by: Neeharika-Sompalli <[email protected]>

* fix review comments, add UnitTest for MerkleDiskFs

Signed-off-by: Neeharika-Sompalli <[email protected]>

* Add unit tests for FreezeHandler

Signed-off-by: Neeharika-Sompalli <[email protected]>

* fix random failing tests because of MockedStatic

Signed-off-by: Neeharika-Sompalli <[email protected]>

* add more tests for codecov

Signed-off-by: Neeharika-Sompalli <[email protected]>

* fix all failing tests, yet to add more tests for code coverage

Signed-off-by: Neeharika-Sompalli <[email protected]>

* remove main method in SignatureVerifier and unused methods as they are not used anywhere

Signed-off-by: Neeharika-Sompalli <[email protected]>

* Address other review comments

Signed-off-by: Neeharika-Sompalli <[email protected]>

* Fix blocker and critical sonar bugs. (#1446)

Signed-off-by: ljianghedera <[email protected]>

* Fix the json map issue.

Signed-off-by: ljianghedera <[email protected]>

* Rename and changes per review comments.

Signed-off-by: ljianghedera <[email protected]>

* Cleanup and more coverage.

Signed-off-by: ljianghedera <[email protected]>

Co-authored-by: Michael Tinker <[email protected]>
Co-authored-by: Anirudh Ghanta <[email protected]>
Co-authored-by: Neeha <[email protected]>
  • Loading branch information
@ljianghedera @anighanta @Neeharika-Sompalli
4 people authored May 26, 2021
1 parent e29cadd commit b14894b
Show file tree
Hide file tree
Showing 5 changed files with 181 additions and 78 deletions.
8 changes: 6 additions & 2 deletions hedera-node/src/main/java/com/hedera/services/fees/TxnRateFeeMultiplierSource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -155,12 +155,16 @@ public void updateMultiplier(Instant consensusNow) {

@Override
public void resetCongestionLevelStarts(Instant[] savedStartTimes) {
congestionLevelStarts = savedStartTimes;
congestionLevelStarts = savedStartTimes.clone();
}

@Override
public Instant[] congestionLevelStarts() {
return congestionLevelStarts;
/* If the Platform is serializing a fast-copy of the MerkleNetworkContext,
and that copy references this object's congestionLevelStarts, we will get
a (transient) ISS if the congestion level changes mid-serialization on one
node but not others. */
return congestionLevelStarts.clone();
}

private boolean ensureConfigUpToDate() {
Expand Down
68 changes: 28 additions & 40 deletions hedera-node/src/main/java/com/hedera/services/state/merkle/MerkleNetworkContext.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import com.hedera.services.fees.FeeMultiplierSource;
import com.hedera.services.state.serdes.DomainSerdes;
import com.hedera.services.state.submerkle.ExchangeRates;
import com.hedera.services.state.submerkle.RichInstant;
import com.hedera.services.state.submerkle.SequenceNumber;
import com.hedera.services.throttles.DeterministicThrottle;
import com.hedera.services.throttling.FunctionalityThrottling;
Expand All @@ -36,7 +35,6 @@
import java.io.IOException;
import java.time.Instant;
import java.util.List;
import java.util.Optional;
import java.util.function.Supplier;

import static com.hedera.services.state.submerkle.RichInstant.fromJava;
Expand All @@ -52,19 +50,19 @@ public class MerkleNetworkContext extends AbstractMerkleLeaf {
static final int RELEASE_0140_VERSION = 3;
static final int MERKLE_VERSION = RELEASE_0140_VERSION;
static final long RUNTIME_CONSTRUCTABLE_ID = 0x8d4aa0f0a968a9f3L;
static final RichInstant[] NO_CONGESTION_STARTS = new RichInstant[0];
static final Instant[] NO_CONGESTION_STARTS = new Instant[0];
static final DeterministicThrottle.UsageSnapshot[] NO_SNAPSHOTS = new DeterministicThrottle.UsageSnapshot[0];

public static final RichInstant UNKNOWN_CONSENSUS_TIME = null;
public static final Instant UNKNOWN_CONSENSUS_TIME = null;

static DomainSerdes serdes = new DomainSerdes();
static Supplier<ExchangeRates> ratesSupplier = ExchangeRates::new;
static Supplier<SequenceNumber> seqNoSupplier = SequenceNumber::new;

private int stateVersion = UNRECORDED_STATE_VERSION;
private RichInstant[] congestionLevelStarts = NO_CONGESTION_STARTS;
private Instant[] congestionLevelStarts = NO_CONGESTION_STARTS;
private ExchangeRates midnightRates;
private RichInstant consensusTimeOfLastHandledTxn;
private Instant consensusTimeOfLastHandledTxn = UNKNOWN_CONSENSUS_TIME;
private SequenceNumber seqNo;
private long lastScannedEntity;
private long entitiesScannedThisSecond = 0L;
Expand All @@ -76,7 +74,7 @@ public MerkleNetworkContext() {
}

public MerkleNetworkContext(
RichInstant consensusTimeOfLastHandledTxn,
Instant consensusTimeOfLastHandledTxn,
SequenceNumber seqNo,
long lastScannedEntity,
ExchangeRates midnightRates
Expand All @@ -88,12 +86,12 @@ public MerkleNetworkContext(
}

public MerkleNetworkContext(
RichInstant consensusTimeOfLastHandledTxn,
Instant consensusTimeOfLastHandledTxn,
SequenceNumber seqNo,
long lastScannedEntity,
ExchangeRates midnightRates,
DeterministicThrottle.UsageSnapshot[] usageSnapshots,
RichInstant[] congestionStartPeriods,
Instant[] congestionStartPeriods,
int stateVersion,
long entitiesScannedThisSecond,
long entitiesTouchedThisSecond
Expand Down Expand Up @@ -137,31 +135,22 @@ public void resetWithSavedSnapshots(FunctionalityThrottling throttling) {
}

public void updateCongestionStartsFrom(FeeMultiplierSource feeMultiplierSource) {
var congestionStarts = feeMultiplierSource.congestionLevelStarts();
if (congestionStarts.length == 0) {
final var congestionStarts = feeMultiplierSource.congestionLevelStarts();
if(null == congestionStarts ) {
congestionLevelStarts = NO_CONGESTION_STARTS;
} else {
congestionLevelStarts = new RichInstant[congestionStarts.length];
for (int i = 0; i < congestionStarts.length; i++) {
congestionLevelStarts[i] = RichInstant.fromJava(congestionStarts[i]);
}
congestionLevelStarts = congestionStarts;
}
}

public void updateWithSavedCongestionStarts(FeeMultiplierSource feeMultiplierSource) {
if (congestionLevelStarts.length > 0) {
Instant[] congestionStarts = new Instant[congestionLevelStarts.length];
for (int i = 0; i < congestionLevelStarts.length; i++) {
if (congestionLevelStarts[i] != null) {
congestionStarts[i] = congestionLevelStarts[i].toJava();
}
}
feeMultiplierSource.resetCongestionLevelStarts(congestionStarts);
feeMultiplierSource.resetCongestionLevelStarts(congestionLevelStarts);
}
}

public void setConsensusTimeOfLastHandledTxn(Instant consensusTimeOfLastHandledTxn) {
this.consensusTimeOfLastHandledTxn = fromJava(consensusTimeOfLastHandledTxn);
this.consensusTimeOfLastHandledTxn = consensusTimeOfLastHandledTxn;
}

public MerkleNetworkContext copy() {
Expand All @@ -180,7 +169,9 @@ public MerkleNetworkContext copy() {

@Override
public void deserialize(SerializableDataInputStream in, int version) throws IOException {
consensusTimeOfLastHandledTxn = serdes.readNullableInstant(in);
final var lastHandleTime = serdes.readNullableInstant(in);
consensusTimeOfLastHandledTxn = (lastHandleTime == null) ? null : lastHandleTime.toJava();

seqNo = seqNoSupplier.get();
seqNo.deserialize(in);
midnightRates = in.readSerializable(true, ratesSupplier);
Expand All @@ -193,16 +184,16 @@ public void deserialize(SerializableDataInputStream in, int version) throws IOEx
var used = in.readLong();
var lastUsed = serdes.readNullableInstant(in);
usageSnapshots[i] = new DeterministicThrottle.UsageSnapshot(
used,
Optional.ofNullable(lastUsed).map(RichInstant::toJava).orElse(null));
used, (lastUsed == null) ? null : lastUsed.toJava());
}
}

int numCongestionStarts = in.readInt();
if (numCongestionStarts > 0) {
congestionLevelStarts = new RichInstant[numCongestionStarts];
congestionLevelStarts = new Instant[numCongestionStarts];
for (int i = 0; i < numCongestionStarts; i++) {
congestionLevelStarts[i] = serdes.readNullableInstant(in);
final var levelStart = serdes.readNullableInstant(in);
congestionLevelStarts[i] = (levelStart == null) ? null : levelStart.toJava();
}
}
}
Expand All @@ -216,7 +207,7 @@ public void deserialize(SerializableDataInputStream in, int version) throws IOEx

@Override
public void serialize(SerializableDataOutputStream out) throws IOException {
serdes.writeNullableInstant(consensusTimeOfLastHandledTxn, out);
serdes.writeNullableInstant(fromJava(consensusTimeOfLastHandledTxn), out);
seqNo.serialize(out);
out.writeSerializable(midnightRates, true);
int n = usageSnapshots.length;
Expand All @@ -228,7 +219,7 @@ public void serialize(SerializableDataOutputStream out) throws IOException {
n = congestionLevelStarts.length;
out.writeInt(n);
for (var congestionStart : congestionLevelStarts) {
serdes.writeNullableInstant(congestionStart, out);
serdes.writeNullableInstant(fromJava(congestionStart), out);
}
out.writeLong(lastScannedEntity);
out.writeLong(entitiesScannedThisSecond);
Expand All @@ -237,7 +228,7 @@ public void serialize(SerializableDataOutputStream out) throws IOException {
}

public Instant consensusTimeOfLastHandledTxn() {
return Optional.ofNullable(consensusTimeOfLastHandledTxn).map(RichInstant::toJava).orElse(null);
return consensusTimeOfLastHandledTxn;
}

public SequenceNumber seqNo() {
Expand Down Expand Up @@ -279,7 +270,7 @@ public String toString() {
for (var snapshot : usageSnapshots) {
sb.append("\n ").append(snapshot.used())
.append(" used (last decision time ")
.append(reprOf(fromJava(snapshot.lastDecisionTime()))).append(")");
.append(reprOf(snapshot.lastDecisionTime())).append(")");
}
sb.append("\n Congestion level start times are ::");
for (var start : congestionLevelStarts) {
Expand Down Expand Up @@ -325,22 +316,19 @@ private void resetUnconditionally(
}
}

private String reprOf(RichInstant consensusTime) {
return Optional.ofNullable(consensusTime)
.map(RichInstant::toJava)
.map(Object::toString)
.orElse("<N/A>");
private String reprOf(Instant consensusTime) {
return consensusTime == null ? "<N/A>" : consensusTime.toString();
}

void setCongestionLevelStarts(RichInstant[] congestionLevelStarts) {
void setCongestionLevelStarts(Instant[] congestionLevelStarts) {
this.congestionLevelStarts = congestionLevelStarts;
}

RichInstant[] getCongestionLevelStarts() {
Instant[] getCongestionLevelStarts() {
return congestionLevelStarts;
}

RichInstant getConsensusTimeOfLastHandledTxn() {
Instant getConsensusTimeOfLastHandledTxn() {
return consensusTimeOfLastHandledTxn;
}

Expand Down
4 changes: 2 additions & 2 deletions hedera-node/src/test/java/com/hedera/services/context/ServicesContextTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ public class ServicesContextTest {
private final NodeId nodeId = new NodeId(false, id);
private static final String recordStreamDir = "somePath/recordStream";

RichInstant consensusTimeOfLastHandledTxn = RichInstant.fromJava(Instant.now());
Instant consensusTimeOfLastHandledTxn = Instant.now();
Platform platform;
SequenceNumber seqNo;
ExchangeRates midnightRates;
Expand Down Expand Up @@ -290,7 +290,7 @@ void delegatesPrimitivesToState() {
inOrder.verify(state).addressBook();
assertEquals(seqNo, actualSeqNo);
assertEquals(midnightRates, actualMidnightRates);
assertEquals(consensusTimeOfLastHandledTxn.toJava(), actualLastHandleTime);
assertEquals(consensusTimeOfLastHandledTxn, actualLastHandleTime);
inOrder.verify(state).topics();
inOrder.verify(state).storage();
inOrder.verify(state).accounts();
Expand Down
22 changes: 21 additions & 1 deletion hedera-node/src/test/java/com/hedera/services/fees/TxnRateFeeMultiplierSourceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.collection.IsIterableContainingInOrder.contains;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotSame;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.mockito.BDDMockito.given;

Expand All @@ -69,13 +70,32 @@ void setUp() {
subject = new TxnRateFeeMultiplierSource(mockProps, throttling);
}

@Test
void makesDefensiveCopyOfCongestionStarts() {
// setup:
final var someInstants = new Instant[] {
Instant.ofEpochSecond(1_234_567L),
Instant.ofEpochSecond(2_234_567L),
};

// given:
subject.resetCongestionLevelStarts(someInstants);

// when:
final var equalNotSameInstants = subject.congestionLevelStarts();

// then:
assertEquals(List.of(someInstants), List.of(equalNotSameInstants));
assertNotSame(someInstants, equalNotSameInstants);
}

@Test
void updatesCongestionStarts() {
// when:
subject.resetCongestionLevelStarts(congestionStarts);

// then:
Assertions.assertSame(congestionStarts, subject.congestionLevelStarts());
Assertions.assertEquals(List.of(congestionStarts), List.of(subject.congestionLevelStarts()));
}

/* MockGlobalDynamicProps has 2 secs for minCongestionPeriod */
Expand Down
Loading

0 comments on commit b14894b

Please sign in to comment.