Skip to content

Commit

Permalink
chore: cherry pick StateNetworkInfo atomic map swap (#17869) (#18025)
Browse files Browse the repository at this point in the history
Signed-off-by: Michael Tinker <[email protected]>
Co-authored-by: Michael Tinker <[email protected]>
  • Loading branch information
derektriley and tinker-michaelj authored Feb 25, 2025
1 parent 59702c7 commit 7d82f35
Show file tree
Hide file tree
Showing 18 changed files with 225 additions and 74 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2023-2024 Hedera Hashgraph, LLC
* Copyright (C) 2023-2025 Hedera Hashgraph, LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -34,6 +34,7 @@
import com.swirlds.state.spi.ReadableKVState;
import edu.umd.cs.findbugs.annotations.NonNull;
import edu.umd.cs.findbugs.annotations.Nullable;
import java.util.Collections;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
Expand All @@ -56,7 +57,14 @@ public class StateNetworkInfo implements NetworkInfo {
*/
private final Roster activeRoster;

private final Map<Long, NodeInfo> nodeInfos;
/**
* Non-final because we need {@code handleTransaction} to be able to swap in an
* updated map atomically, without giving a pre-handle thread a temporary view of
* an empty map. (Note that {@code handleTransaction}'s updates will only change
* <i>metadata</i> of nodes, but not the set of nodes itself; so pre-handle threads
* can use any version of the map to test for address book membership.)
*/
private volatile Map<Long, NodeInfo> nodeInfos;

/**
* Constructs a new network information provider from the given state, roster, selfID, and configuration provider.
Expand Down Expand Up @@ -113,8 +121,7 @@ public boolean containsNode(final long nodeId) {

@Override
public void updateFrom(@NonNull final State state) {
nodeInfos.clear();
nodeInfos.putAll(nodeInfosFrom(state));
nodeInfos = nodeInfosFrom(state);
}

/**
Expand Down Expand Up @@ -150,6 +157,6 @@ private Map<Long, NodeInfo> nodeInfosFrom(@NonNull final State state) {
log.warn("Node {} not found in node store", rosterEntry.nodeId());
}
}
return nodeInfos;
return Collections.unmodifiableMap(nodeInfos);
}
}
5 changes: 3 additions & 2 deletions hedera-node/test-clients/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,8 @@ tasks.register<Test>("testEmbedded") {
.joinToString("|")
useJUnitPlatform {
includeTags(
if (ciTagExpression.isBlank()) "none()|!(RESTART|ND_RECONNECT|UPGRADE|REPEATABLE)"
if (ciTagExpression.isBlank())
"none()|!(RESTART|ND_RECONNECT|UPGRADE|REPEATABLE|ONLY_SUBPROCESS)"
else "(${ciTagExpression}|STREAM_VALIDATION|LOG_VALIDATION)&(!INTEGRATION)"
)
}
Expand Down Expand Up @@ -245,7 +246,7 @@ tasks.register<Test>("testRepeatable") {
useJUnitPlatform {
includeTags(
if (ciTagExpression.isBlank())
"none()|!(RESTART|ND_RECONNECT|UPGRADE|EMBEDDED|NOT_REPEATABLE)"
"none()|!(RESTART|ND_RECONNECT|UPGRADE|EMBEDDED|NOT_REPEATABLE|ONLY_SUBPROCESS)"
else "(${ciTagExpression}|STREAM_VALIDATION|LOG_VALIDATION)&(!INTEGRATION)"
)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2023-2024 Hedera Hashgraph, LLC
* Copyright (C) 2023-2025 Hedera Hashgraph, LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -34,6 +34,11 @@ private TestTags() {
* integration tests of the app workflows (e.g., ingest, pre-handle, handle) and services.
*/
public static final String INTEGRATION = "INTEGRATION";
/**
* Tags a test that <b>must</b> be run in subprocess mode, generally because it
* depends on actual gossip occurring.
*/
public static final String ONLY_SUBPROCESS = "ONLY_SUBPROCESS";
/**
* Tags a test that <b>must</b> be run in embedded mode, either because it directly
* submits duplicate or invalid transactions to non-default nodes; or because it
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,9 @@ public String toString() {
private ThreadPoolExecutor finalizingExecutor;
private CompletableFuture<Void> finalizingFuture;

@Nullable
private Map<String, String> setupOverrides;

/**
* If non-null, the non-remote network to target with this spec.
*/
Expand Down Expand Up @@ -695,6 +698,27 @@ public boolean tryReinitializingFees() {
return false;
}

/**
* Add properties that will be given priority in the spec's {@link HapiSpecSetup}.
* @param props the properties to add
* @return this
*/
private HapiSpec withPrioritySetup(@Nullable final Map<String, String> props) {
if (props != null) {
setupOverrides = props;
}
return this;
}

/**
* Finalizes the setup properties for this spec.
*/
private void finalizeSetupProperties() {
if (setupOverrides != null) {
hapiSetup.addOverrides(setupOverrides);
}
}

private boolean init() {
if (targetNetwork == null) {
targetNetwork = RemoteNetwork.newRemoteNetwork(hapiSetup.nodes(), clientsFor(hapiSetup));
Expand Down Expand Up @@ -1083,29 +1107,48 @@ public static Def.Setup hapiSpec(String name, List<String> propertiesToPreserve)
*/
public static Stream<DynamicTest> hapiTest(@NonNull final SpecOperation... ops) {
return propertyPreservingHapiTest(
Optional.ofNullable(PROPERTIES_TO_PRESERVE.get()).orElse(emptyList()), ops);
Optional.ofNullable(PROPERTIES_TO_PRESERVE.get()).orElse(emptyList()), null, ops);
}

/**
* Creates dynamic tests derived from with the given operations and {@link HapiSpecSetup} overrides, preserving
* any network properties bound to the thread by a {@link LeakyHapiTest} test factory.
*
* @param setupOverrides the setup overrides
* @param ops the operations
* @return a {@link Stream} of {@link DynamicTest}s
*/
public static Stream<DynamicTest> customizedHapiTest(
@NonNull final Map<String, String> setupOverrides, @NonNull final SpecOperation... ops) {
requireNonNull(setupOverrides);
return propertyPreservingHapiTest(
Optional.ofNullable(PROPERTIES_TO_PRESERVE.get()).orElse(emptyList()), setupOverrides, ops);
}

/**
* Creates dynamic tests derived from with the given operations, ensuring the listed properties are
* restored to their original values after running the tests.
*
* @param propertiesToPreserve the properties to preserve
* @param ops the operations
* @param setupOverrides the setup overrides, if any
* @param ops the operations
* @return a {@link Stream} of {@link DynamicTest}s
*/
public static Stream<DynamicTest> propertyPreservingHapiTest(
@NonNull final List<String> propertiesToPreserve, @NonNull final SpecOperation... ops) {
private static Stream<DynamicTest> propertyPreservingHapiTest(
@NonNull final List<String> propertiesToPreserve,
@Nullable final Map<String, String> setupOverrides,
@NonNull final SpecOperation... ops) {
requireNonNull(propertiesToPreserve);
return Stream.of(DynamicTest.dynamicTest(
AS_WRITTEN_DISPLAY_NAME,
targeted(new HapiSpec(
SPEC_NAME.get(),
HapiSpecSetup.setupFrom(HapiSpecSetup.getDefaultPropertySource()),
new SpecOperation[0],
new SpecOperation[0],
ops,
propertiesToPreserve))));
SPEC_NAME.get(),
HapiSpecSetup.setupFrom(HapiSpecSetup.getDefaultPropertySource()),
new SpecOperation[0],
new SpecOperation[0],
ops,
propertiesToPreserve)
.withPrioritySetup(setupOverrides))));
}

public static DynamicTest namedHapiTest(String name, @NonNull final SpecOperation... ops) {
Expand Down Expand Up @@ -1147,7 +1190,7 @@ public static void doTargetSpec(@NonNull final HapiSpec spec, @NonNull final Hed
// directly from the network's HederaNode instances instead of this "nodes" property
final var specNodes =
targetNetwork.nodes().stream().map(HederaNode::hapiSpecInfo).collect(joining(","));
spec.addOverrideProperties(Map.of("nodes", specNodes));
spec.addOverrideProperties(Map.of("nodes", specNodes, "memo.useSpecName", "true"));

if (targetNetwork instanceof EmbeddedNetwork embeddedNetwork) {
final Map<String, String> overrides;
Expand All @@ -1165,6 +1208,8 @@ public static void doTargetSpec(@NonNull final HapiSpec spec, @NonNull final Hed
spec.setKeyGenerator(requireNonNull(REPEATABLE_KEY_GENERATOR.get()));
}
}

spec.finalizeSetupProperties();
}

public HapiSpec(String name, SpecOperation[] ops) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2020-2024 Hedera Hashgraph, LLC
* Copyright (C) 2020-2025 Hedera Hashgraph, LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -31,9 +31,21 @@
import com.hedera.services.bdd.spec.props.MapPropertySource;
import com.hedera.services.bdd.spec.props.NodeConnectInfo;
import com.hedera.services.bdd.spec.transactions.HapiTxnOp;
import com.hederahashgraph.api.proto.java.*;
import com.hederahashgraph.api.proto.java.AccountID;
import com.hederahashgraph.api.proto.java.ContractID;
import com.hederahashgraph.api.proto.java.Duration;
import com.hederahashgraph.api.proto.java.FileID;
import com.hederahashgraph.api.proto.java.RealmID;
import com.hederahashgraph.api.proto.java.ResponseCodeEnum;
import com.hederahashgraph.api.proto.java.ServiceEndpoint;
import com.hederahashgraph.api.proto.java.ShardID;
import edu.umd.cs.findbugs.annotations.NonNull;
import java.util.*;
import java.util.Collections;
import java.util.EnumSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.SplittableRandom;
import java.util.function.Function;
import java.util.stream.Collectors;
import java.util.stream.Stream;
Expand Down Expand Up @@ -129,6 +141,14 @@ public void addOverrides(@NonNull final Map<String, String> props) {
this.props = HapiPropertySource.inPriorityOrder(new MapPropertySource(props), this.props);
}

/**
* Returns whether the default transaction memo should be the name of the {@link HapiSpec}
* submitting the transaction.
*/
public boolean useSpecName() {
return props.getBoolean("memo.useSpecName");
}

public FileID addressBookId() {
return props.getFile("address.book.id");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@
* Used by a {@link HapiSpec} to create transactions for submission to its target network.
*/
public class TxnFactory {
private static final int MEMO_PREFIX_LIMIT = 100;
private static final double TXN_ID_SAMPLE_PROBABILITY = 1.0 / 500;

private final HapiSpecSetup setup;
Expand Down Expand Up @@ -153,7 +154,8 @@ public Transaction.Builder getReadyToSign(
@Nullable final BodyMutation modification,
@Nullable final HapiSpec spec) {
requireNonNull(bodySpec);
final var composedBodySpec = defaultBodySpec().andThen(bodySpec);
final var composedBodySpec =
defaultBodySpec(spec == null ? null : spec.getName()).andThen(bodySpec);
var bodyBuilder = TransactionBody.newBuilder();
composedBodySpec.accept(bodyBuilder);
if (modification != null) {
Expand Down Expand Up @@ -189,12 +191,14 @@ public <T, B extends Message.Builder> T body(@NonNull final Class<T> tClass, @No
return (T) opBuilder.build();
}

private Consumer<TransactionBody.Builder> defaultBodySpec() {
private Consumer<TransactionBody.Builder> defaultBodySpec(@Nullable final String specName) {
final var defaultTxnId = nextTxnId.get();
if (r.nextDouble() < TXN_ID_SAMPLE_PROBABILITY) {
sampleTxnId.set(defaultTxnId);
}
final var memoToUse = (setup.isMemoUTF8() == TRUE) ? setup.defaultUTF8memo() : setup.defaultMemo();
final var memoToUse = (specName != null && setup.useSpecName())
? specName.substring(0, Math.min(specName.length(), MEMO_PREFIX_LIMIT))
: (setup.isMemoUTF8() == TRUE ? setup.defaultUTF8memo() : setup.defaultMemo());
return builder -> builder.setTransactionID(defaultTxnId)
.setMemo(memoToUse)
.setTransactionFee(setup.defaultFee())
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2024 Hedera Hashgraph, LLC
* Copyright (C) 2024-2025 Hedera Hashgraph, LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -19,6 +19,7 @@
import static com.hedera.node.app.hapi.utils.EthSigsUtils.recoverAddressFromPubKey;
import static com.hedera.services.bdd.junit.TestTags.CRYPTO;
import static com.hedera.services.bdd.spec.HapiPropertySource.asSolidityAddress;
import static com.hedera.services.bdd.spec.HapiSpec.customizedHapiTest;
import static com.hedera.services.bdd.spec.HapiSpec.hapiTest;
import static com.hedera.services.bdd.spec.assertions.AccountInfoAsserts.accountWith;
import static com.hedera.services.bdd.spec.assertions.TransactionRecordAsserts.recordWith;
Expand Down Expand Up @@ -71,6 +72,7 @@
import com.hederahashgraph.api.proto.java.TransferList;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.concurrent.atomic.AtomicLong;
import java.util.concurrent.atomic.AtomicReference;
import java.util.stream.Stream;
Expand Down Expand Up @@ -103,7 +105,8 @@ public class AutoAccountCreationUnlimitedAssociationsSuite {
final Stream<DynamicTest> autoAccountCreationsUnlimitedAssociationHappyPath() {
final var creationTime = new AtomicLong();
final long transferFee = 188608L;
return hapiTest(
return customizedHapiTest(
Map.of("memo.useSpecName", "false"),
newKeyNamed(VALID_ALIAS),
cryptoCreate(CIVILIAN).balance(10 * ONE_HBAR),
cryptoCreate(PAYER).balance(10 * ONE_HBAR),
Expand Down Expand Up @@ -158,7 +161,8 @@ final Stream<DynamicTest> autoAccountCreationsUnlimitedAssociationHappyPath() {
final Stream<DynamicTest> autoAccountCreationsUnlimitedAssociationsDisabled() {
final var creationTime = new AtomicLong();
final long transferFee = 188608L;
return hapiTest(
return customizedHapiTest(
Map.of("memo.useSpecName", "false"),
overriding("entities.unlimitedAutoAssociationsEnabled", FALSE),
newKeyNamed(VALID_ALIAS),
cryptoCreate(CIVILIAN).balance(10 * ONE_HBAR),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2022-2024 Hedera Hashgraph, LLC
* Copyright (C) 2022-2025 Hedera Hashgraph, LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -23,6 +23,7 @@
import static com.hedera.services.bdd.junit.TestTags.CRYPTO;
import static com.hedera.services.bdd.spec.HapiPropertySource.asContractString;
import static com.hedera.services.bdd.spec.HapiPropertySource.asSolidityAddress;
import static com.hedera.services.bdd.spec.HapiSpec.customizedHapiTest;
import static com.hedera.services.bdd.spec.HapiSpec.hapiTest;
import static com.hedera.services.bdd.spec.assertions.AccountDetailsAsserts.accountDetailsWith;
import static com.hedera.services.bdd.spec.assertions.AccountInfoAsserts.accountWith;
Expand Down Expand Up @@ -159,6 +160,7 @@
import java.math.BigInteger;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.OptionalLong;
import java.util.concurrent.atomic.AtomicLong;
import java.util.concurrent.atomic.AtomicReference;
Expand Down Expand Up @@ -192,7 +194,8 @@ final Stream<DynamicTest> autoAssociationPropertiesWorkAsExpected() {
final var payerBalance = 100 * ONE_HUNDRED_HBARS;
final var updateWithExpiredAccount = "updateWithExpiredAccount";
final var baseFee = 0.000214;
return hapiTest(
return customizedHapiTest(
Map.of("memo.useSpecName", "false"),
overridingTwo(
"ledger.maxAutoAssociations", "100",
"ledger.autoRenewPeriod.minDuration", "1"),
Expand Down
Loading

0 comments on commit 7d82f35

Please sign in to comment.