Skip to content

Commit

Permalink
chore: add node details/address book export validation in `DabEnabled…
Browse files Browse the repository at this point in the history 
…UpgradeTest` (#15764)

Signed-off-by: Michael Tinker <[email protected]>
  • Loading branch information
@tinker-michaelj
tinker-michaelj authored Oct 6, 2024
1 parent d58338a commit cdce866
Show file tree
Hide file tree
Showing 4 changed files with 88 additions and 29 deletions.
48 changes: 48 additions & 0 deletions ...a-node/test-clients/src/main/java/com/hedera/services/bdd/spec/transactions/TxnUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import static com.hedera.services.bdd.spec.utilops.CustomSpecAssert.allRunFor;
import static com.hedera.services.bdd.suites.HapiSuite.DEFAULT_PAYER;
import static com.hedera.services.bdd.suites.HapiSuite.FUNDING;
import static com.hederahashgraph.api.proto.java.HederaFunctionality.FileUpdate;
import static com.hederahashgraph.api.proto.java.ResponseCodeEnum.BUSY;
import static com.hederahashgraph.api.proto.java.ResponseCodeEnum.PLATFORM_TRANSACTION_NOT_CREATED;
import static com.swirlds.common.stream.LinkedObjectStreamUtilities.getPeriod;
Expand All @@ -39,6 +40,7 @@
import static java.util.stream.Collectors.joining;
import static java.util.stream.Collectors.toList;
import static java.util.stream.Collectors.toMap;
import static java.util.stream.Collectors.toSet;

import com.google.common.primitives.Ints;
import com.google.common.primitives.Longs;
Expand All @@ -47,6 +49,7 @@
import com.google.protobuf.TextFormat;
import com.hedera.node.app.hapi.fees.usage.SigUsage;
import com.hedera.node.app.hapi.utils.fee.SigValueObj;
import com.hedera.node.app.hapi.utils.forensics.RecordStreamEntry;
import com.hedera.pbj.runtime.JsonCodec;
import com.hedera.services.bdd.spec.HapiPropertySource;
import com.hedera.services.bdd.spec.HapiSpec;
Expand All @@ -60,6 +63,7 @@
import com.hedera.services.bdd.spec.transactions.crypto.HapiCryptoTransfer;
import com.hedera.services.bdd.spec.utilops.streams.InterruptibleRunnable;
import com.hedera.services.bdd.suites.contract.Utils;
import com.hedera.services.stream.proto.RecordStreamItem;
import com.hederahashgraph.api.proto.java.AccountAmount;
import com.hederahashgraph.api.proto.java.AccountID;
import com.hederahashgraph.api.proto.java.ContractID;
Expand Down Expand Up @@ -103,7 +107,9 @@
import java.util.SplittableRandom;
import java.util.UUID;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.function.BiPredicate;
import java.util.function.Function;
import java.util.function.Predicate;
import java.util.regex.Pattern;
import java.util.stream.IntStream;
import java.util.stream.Stream;
Expand Down Expand Up @@ -783,4 +789,46 @@ public static String resourceAsString(@NonNull final String loc) {
throw new UncheckedIOException(e);
}
}

/**
* Predicate that checks if a given {@link RecordStreamItem} is a system file update relative to the given spec.
* @param spec the spec to use for the check
* @param item the item to check
* @return {@code true} if the item is a system file update, {@code false} otherwise
*/
public static boolean isSysFileUpdate(@NonNull final HapiSpec spec, @NonNull final RecordStreamItem item) {
final var firstUserNum = spec.startupProperties().getLong("hedera.firstUserEntity");
return filterForSysFileUpdate(spec, item, id -> id.getFileNum() < firstUserNum);
}

/**
* Returns a predicate that checks if a given {@link RecordStreamItem} is a system file update targeting a
* particular system file relative to the given spec.
* @param sysFileProperties the name(s) of the system file properties to screen for
* @return a predicate testing if the item is a system file update to a given file
*/
public static BiPredicate<HapiSpec, RecordStreamItem> sysFileUpdateTo(@NonNull final String... sysFileProperties) {
requireNonNull(sysFileProperties);
return (spec, item) -> {
final var sysFileNums = Arrays.stream(sysFileProperties)
.map(spec.startupProperties()::getLong)
.collect(toSet());
return filterForSysFileUpdate(spec, item, id -> sysFileNums.contains(id.getFileNum()));
};
}

private static boolean filterForSysFileUpdate(
@NonNull final HapiSpec spec,
@NonNull final RecordStreamItem item,
@NonNull final Predicate<FileID> idFilter) {
final var txnId = item.getRecord().getTransactionID();
final var sysAdminNum = spec.startupProperties().getLong("accounts.systemAdmin");
if (txnId.getAccountID().getAccountNum() != sysAdminNum) {
return false;
} else {
final var entry = RecordStreamEntry.from(item);
return entry.function() == FileUpdate
&& idFilter.test(entry.body().getFileUpdate().getFileID());
}
}
}
7 changes: 7 additions & 0 deletions ...n/java/com/hedera/services/bdd/spec/utilops/streams/assertions/VisibleItemsValidator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,13 @@
* A validator for visible items, called once all items have been collected.
*/
public interface VisibleItemsValidator {
/**
* A no-op validator to clarify intent when constructing a {@link SelectedItemsAssertion} that should pass whenever
* its managing {@link EventualRecordStreamAssertion} discovers the expected number of selected items within the
* timeout.
*/
VisibleItemsValidator EXISTENCE_ONLY_VALIDATOR = (spec, allItems) -> {};

/**
* Asserts that the visible items are valid.
*
Expand Down
39 changes: 12 additions & 27 deletions ...st-clients/src/main/java/com/hedera/services/bdd/suites/hip993/SystemFileExportsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import static com.hedera.services.bdd.spec.queries.QueryVerbs.getFileContents;
import static com.hedera.services.bdd.spec.transactions.TxnUtils.randomUtf8Bytes;
import static com.hedera.services.bdd.spec.transactions.TxnUtils.resourceAsString;
import static com.hedera.services.bdd.spec.transactions.TxnUtils.sysFileUpdateTo;
import static com.hedera.services.bdd.spec.transactions.TxnVerbs.contractCall;
import static com.hedera.services.bdd.spec.transactions.TxnVerbs.contractCreate;
import static com.hedera.services.bdd.spec.transactions.TxnVerbs.cryptoCreate;
Expand Down Expand Up @@ -74,15 +75,14 @@
import com.hedera.hapi.node.base.FileID;
import com.hedera.hapi.node.base.NodeAddressBook;
import com.hedera.hapi.node.base.ServiceEndpoint;
import com.hedera.node.app.hapi.utils.forensics.RecordStreamEntry;
import com.hedera.pbj.runtime.ParseException;
import com.hedera.pbj.runtime.io.buffer.Bytes;
import com.hedera.services.bdd.junit.GenesisHapiTest;
import com.hedera.services.bdd.spec.HapiSpec;
import com.hedera.services.bdd.spec.transactions.TxnUtils;
import com.hedera.services.bdd.spec.utilops.grouping.SysFileLookups;
import com.hedera.services.bdd.spec.utilops.streams.assertions.VisibleItems;
import com.hedera.services.bdd.spec.utilops.streams.assertions.VisibleItemsValidator;
import com.hedera.services.stream.proto.RecordStreamItem;
import com.hederahashgraph.api.proto.java.CurrentAndNextFeeSchedule;
import com.hederahashgraph.api.proto.java.ServicesConfigurationList;
import com.hederahashgraph.api.proto.java.ThrottleDefinitions;
Expand Down Expand Up @@ -124,7 +124,9 @@ final Stream<DynamicTest> syntheticNodeDetailsUpdateHappensAtUpgradeBoundary() {
final AtomicReference<Map<Long, X509Certificate>> gossipCertificates = new AtomicReference<>();
return hapiTest(
recordStreamMustIncludePassFrom(selectedItems(
nodeDetailsExportValidator(grpcCertHashes, gossipCertificates), 1, this::isSysFileUpdate)),
nodeDetailsExportValidator(grpcCertHashes, gossipCertificates),
1,
sysFileUpdateTo("files.nodeDetails"))),
given(() -> gossipCertificates.set(generateCertificates(CLASSIC_HAPI_TEST_NETWORK_SIZE))),
// This is the genesis transaction
cryptoCreate("firstUser"),
Expand All @@ -147,9 +149,7 @@ final Stream<DynamicTest> syntheticAddressBookUpdateHappensAtUpgradeBoundary() {
final AtomicReference<Map<Long, X509Certificate>> gossipCertificates = new AtomicReference<>();
return hapiTest(
recordStreamMustIncludePassFrom(selectedItems(
addressBookExportValidator("files.addressBook", grpcCertHashes, gossipCertificates),
2,
this::isSysFileUpdate)),
addressBookExportValidator("files.addressBook", grpcCertHashes), 2, TxnUtils::isSysFileUpdate)),
given(() -> gossipCertificates.set(generateCertificates(CLASSIC_HAPI_TEST_NETWORK_SIZE))),
// This is the genesis transaction
cryptoCreate("firstUser"),
Expand All @@ -175,7 +175,7 @@ final Stream<DynamicTest> syntheticFeeSchedulesUpdateHappensAtUpgradeBoundary()
sysFileExportValidator(
"files.feeSchedules", upgradeFeeSchedules, SystemFileExportsTest::parseFeeSchedule),
3,
this::isSysFileUpdate)),
TxnUtils::isSysFileUpdate)),
// This is the genesis transaction
sourcingContextual(spec -> overridingTwo(
"networkAdmin.upgradeSysFilesLoc",
Expand Down Expand Up @@ -220,7 +220,7 @@ final Stream<DynamicTest> syntheticThrottlesUpdateHappensAtUpgradeBoundary() thr
upgradeThrottleDefs,
SystemFileExportsTest::parseThrottleDefs),
3,
this::isSysFileUpdate)),
TxnUtils::isSysFileUpdate)),
// This is the genesis transaction
sourcingContextual(spec -> overridingTwo(
"networkAdmin.upgradeSysFilesLoc",
Expand Down Expand Up @@ -261,7 +261,7 @@ final Stream<DynamicTest> syntheticPropertyOverridesUpdateHappensAtUpgradeBounda
upgradePropOverrides,
SystemFileExportsTest::parseConfigList),
3,
this::isSysFileUpdate)),
TxnUtils::isSysFileUpdate)),
// This is the genesis transaction
sourcingContextual(spec -> overriding(
"networkAdmin.upgradeSysFilesLoc",
Expand Down Expand Up @@ -296,7 +296,7 @@ final Stream<DynamicTest> syntheticPropertyOverridesUpdateCanBeEmptyFile() {
ServicesConfigurationList.getDefaultInstance(),
SystemFileExportsTest::parseConfigList),
3,
this::isSysFileUpdate)),
TxnUtils::isSysFileUpdate)),
// This is the genesis transaction
sourcingContextual(spec -> overridingTwo(
"networkAdmin.upgradeSysFilesLoc",
Expand Down Expand Up @@ -337,7 +337,7 @@ final Stream<DynamicTest> syntheticPermissionOverridesUpdateHappensAtUpgradeBoun
upgradePermissionOverrides,
SystemFileExportsTest::parseConfigList),
3,
this::isSysFileUpdate)),
TxnUtils::isSysFileUpdate)),
// This is the genesis transaction
sourcingContextual(spec -> overriding(
"networkAdmin.upgradeSysFilesLoc",
Expand Down Expand Up @@ -467,9 +467,7 @@ private static VisibleItemsValidator nodeDetailsExportValidator(
}

private static VisibleItemsValidator addressBookExportValidator(
@NonNull final String fileNumProperty,
@NonNull final byte[][] grpcCertHashes,
@NonNull final AtomicReference<Map<Long, X509Certificate>> gossipCertificates) {
@NonNull final String fileNumProperty, @NonNull final byte[][] grpcCertHashes) {
return (spec, records) -> {
final var items = records.get(SELECTED_ITEMS_KEY);
assertNotNull(items, "No post-upgrade txn found");
Expand Down Expand Up @@ -564,17 +562,4 @@ private static List<ServiceEndpoint> endpointsFor(final int i) {
return List.of(asDnsServiceEndpoint("host" + i + ":" + (80 + i)));
}
}

private boolean isSysFileUpdate(@NonNull final HapiSpec spec, @NonNull final RecordStreamItem item) {
final var txnId = item.getRecord().getTransactionID();
final var sysAdminNum = spec.startupProperties().getLong("accounts.systemAdmin");
final var firstUserNum = spec.startupProperties().getLong("hedera.firstUserEntity");
if (txnId.getAccountID().getAccountNum() != sysAdminNum) {
return false;
} else {
final var entry = RecordStreamEntry.from(item);
return entry.function() == FileUpdate
&& entry.body().getFileUpdate().getFileID().getFileNum() < firstUserNum;
}
}
}
23 changes: 21 additions & 2 deletions ...src/main/java/com/hedera/services/bdd/suites/regression/system/DabEnabledUpgradeTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,21 @@
import static com.hedera.services.bdd.spec.HapiSpec.hapiTest;
import static com.hedera.services.bdd.spec.dsl.operations.transactions.TouchBalancesOperation.touchBalanceOf;
import static com.hedera.services.bdd.spec.queries.QueryVerbs.getVersionInfo;
import static com.hedera.services.bdd.spec.transactions.TxnUtils.sysFileUpdateTo;
import static com.hedera.services.bdd.spec.transactions.TxnVerbs.cryptoCreate;
import static com.hedera.services.bdd.spec.transactions.TxnVerbs.cryptoTransfer;
import static com.hedera.services.bdd.spec.transactions.TxnVerbs.nodeCreate;
import static com.hedera.services.bdd.spec.transactions.TxnVerbs.nodeDelete;
import static com.hedera.services.bdd.spec.transactions.TxnVerbs.nodeUpdate;
import static com.hedera.services.bdd.spec.transactions.crypto.HapiCryptoTransfer.tinyBarsFromTo;
import static com.hedera.services.bdd.spec.utilops.UtilVerbs.ensureStakingActivated;
import static com.hedera.services.bdd.spec.utilops.UtilVerbs.given;
import static com.hedera.services.bdd.spec.utilops.UtilVerbs.recordStreamMustIncludePassFrom;
import static com.hedera.services.bdd.spec.utilops.UtilVerbs.selectedItems;
import static com.hedera.services.bdd.spec.utilops.UtilVerbs.sourcing;
import static com.hedera.services.bdd.spec.utilops.UtilVerbs.validateUpgradeAddressBooks;
import static com.hedera.services.bdd.spec.utilops.UtilVerbs.waitUntilStartOfNextStakingPeriod;
import static com.hedera.services.bdd.spec.utilops.streams.assertions.VisibleItemsValidator.EXISTENCE_ONLY_VALIDATOR;
import static com.hedera.services.bdd.suites.HapiSuite.DEFAULT_PAYER;
import static com.hedera.services.bdd.suites.HapiSuite.FUNDING;
import static com.hedera.services.bdd.suites.HapiSuite.ONE_BILLION_HBARS;
Expand Down Expand Up @@ -134,11 +139,15 @@ class WithUnchangedNodes {
final Stream<DynamicTest> sameNodesTest() {
final AtomicReference<SemanticVersion> startVersion = new AtomicReference<>();
return hapiTest(
recordStreamMustIncludePassFrom(selectedItems(
EXISTENCE_ONLY_VALIDATOR, 2, sysFileUpdateTo("files.nodeDetails", "files.addressBook"))),
getVersionInfo().exposingServicesVersionTo(startVersion::set),
prepareFakeUpgrade(),
validateUpgradeAddressBooks(DabEnabledUpgradeTest::hasClassicAddressMetadata),
upgradeToNextConfigVersion(),
assertExpectedConfigVersion(startVersion::get));
assertExpectedConfigVersion(startVersion::get),
// Ensure we have a post-upgrade transaction to trigger system file exports
cryptoCreate("somebodyNew"));
}
}

Expand All @@ -157,6 +166,8 @@ static void beforeAll(@NonNull final TestLifecycle testLifecycle) {
@DisplayName("exports an address book without node1 and pays its stake no rewards")
final Stream<DynamicTest> removedNodeTest() {
return hapiTest(
recordStreamMustIncludePassFrom(selectedItems(
EXISTENCE_ONLY_VALIDATOR, 2, sysFileUpdateTo("files.nodeDetails", "files.addressBook"))),
prepareFakeUpgrade(),
validateUpgradeAddressBooks(
addressBook -> assertThat(nodeIdsFrom(addressBook)).containsExactlyInAnyOrder(0L, 2L, 3L)),
Expand Down Expand Up @@ -192,6 +203,8 @@ static void beforeAll(@NonNull final TestLifecycle testLifecycle) {
@DisplayName("exports an address book without node id3 and pays its stake no rewards")
final Stream<DynamicTest> removedNodeTest() {
return hapiTest(
recordStreamMustIncludePassFrom(selectedItems(
EXISTENCE_ONLY_VALIDATOR, 2, sysFileUpdateTo("files.nodeDetails", "files.addressBook"))),
prepareFakeUpgrade(),
validateUpgradeAddressBooks(
addressBook -> assertThat(nodeIdsFrom(addressBook)).containsExactlyInAnyOrder(0L, 2L)),
Expand Down Expand Up @@ -222,11 +235,15 @@ static void beforeAll(@NonNull final TestLifecycle testLifecycle) throws Certifi
@DisplayName("exports an address book with node id4")
final Stream<DynamicTest> exportedAddressBookIncludesNodeId4() {
return hapiTest(
recordStreamMustIncludePassFrom(selectedItems(
EXISTENCE_ONLY_VALIDATOR, 2, sysFileUpdateTo("files.nodeDetails", "files.addressBook"))),
prepareFakeUpgrade(),
// node4 was not active before this the upgrade, so it could not have written a config.txt
validateUpgradeAddressBooks(exceptNodeIds(4L), addressBook -> assertThat(nodeIdsFrom(addressBook))
.contains(4L)),
upgradeToNextConfigVersion(FakeNmt.addNode(4L, DAB_GENERATED)));
upgradeToNextConfigVersion(FakeNmt.addNode(4L, DAB_GENERATED)),
// Ensure we have a post-upgrade transaction to trigger system file exports
cryptoCreate("somebodyNew"));
}
}

Expand Down Expand Up @@ -284,6 +301,8 @@ static void beforeAll(@NonNull final TestLifecycle testLifecycle) throws Certifi
@DisplayName("exported address book reflects only edits before prepare upgrade")
final Stream<DynamicTest> exportedAddressBookReflectsOnlyEditsBeforePrepareUpgrade() {
return hapiTest(
recordStreamMustIncludePassFrom(selectedItems(
EXISTENCE_ONLY_VALIDATOR, 2, sysFileUpdateTo("files.nodeDetails", "files.addressBook"))),
prepareFakeUpgrade(),
// Now make some changes that should not be incorporated in this upgrade
nodeDelete("5"),
Expand Down

0 comments on commit cdce866

Please sign in to comment.