Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: In CI @HapiTests, use spec name as default memo #17870

Merged
merged 9 commits into from
Feb 19, 2025
Merged
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
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
Expand Up @@ -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
Loading