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

refactor: adapt StatsSigningTestingTool to work with Bytes wrapper for transactions #17144

Open
wants to merge 31 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
63edb17
refactor: consumer for system transactions
mustafauzunn Dec 19, 2024
50669f7
refactor: adapt StatsSigningTestingTool to work with Bytes wrapper fo…
IvanKavaldzhiev Dec 20, 2024
fd7222c
Merge remote-tracking branch 'origin/develop' into 16871-adapt-StatsS…
IvanKavaldzhiev Dec 20, 2024
a7d6e22
style: restore headers
IvanKavaldzhiev Dec 20, 2024
b4dcec7
Merge remote-tracking branch 'origin/16703-refactor-consumer-system-t…
IvanKavaldzhiev Dec 20, 2024
8478fd9
Merge branch 'develop' into 16703-refactor-consumer-system-transactions
mustafauzunn Dec 20, 2024
2c6d151
Merge remote-tracking branch 'origin/16703-refactor-consumer-system-t…
IvanKavaldzhiev Dec 20, 2024
b67ca58
Merge remote-tracking branch 'origin/develop' into 16871-adapt-StatsS…
IvanKavaldzhiev Dec 20, 2024
2145caa
nit: resolve PR comments
IvanKavaldzhiev Dec 20, 2024
9315b1c
nit: resolve PR comments
IvanKavaldzhiev Dec 20, 2024
c26e80d
Merge remote-tracking branch 'origin/main' into 16871-adapt-StatsSign…
IvanKavaldzhiev Dec 26, 2024
a3d2c8b
nit: resolve PR comment and change system transaction differentiation
IvanKavaldzhiev Dec 27, 2024
7520540
nit: fix build
IvanKavaldzhiev Dec 27, 2024
c82d849
nit: resolve PR comment
IvanKavaldzhiev Dec 27, 2024
dcb186e
Merge remote-tracking branch 'origin/main' into 16871-adapt-StatsSign…
IvanKavaldzhiev Dec 27, 2024
3d56d76
Merge branch 'main' into 16871-adapt-StatsSigningTestingTool-to-work-…
rbarkerSL Jan 2, 2025
4e9ef8b
Merge remote-tracking branch 'origin/main' into 16871-adapt-StatsSign…
IvanKavaldzhiev Jan 6, 2025
0cc6405
Merge remote-tracking branch 'origin/16871-adapt-StatsSigningTestingT…
IvanKavaldzhiev Jan 6, 2025
f3fd9b8
Merge remote-tracking branch 'origin/main' into 16871-adapt-StatsSign…
IvanKavaldzhiev Jan 7, 2025
09bba4d
refactor: consume converted system transaction in callback instead of…
IvanKavaldzhiev Jan 7, 2025
734ed9a
Merge remote-tracking branch 'origin/main' into 16871-adapt-StatsSign…
IvanKavaldzhiev Jan 7, 2025
90f12bc
style: spotless apply and remove unnecessary check
IvanKavaldzhiev Jan 7, 2025
c91aa49
style: spotless apply
IvanKavaldzhiev Jan 7, 2025
9877ca8
Merge remote-tracking branch 'origin/main' into 16871-adapt-StatsSign…
IvanKavaldzhiev Jan 8, 2025
e8e5eae
nit: add final declarations and comments for elaborating business logic
IvanKavaldzhiev Jan 8, 2025
24c549a
Merge remote-tracking branch 'origin/main' into 16871-adapt-StatsSign…
IvanKavaldzhiev Jan 9, 2025
8a809cf
refactor: simplify logic for distinguishing between system and applic…
IvanKavaldzhiev Jan 10, 2025
fd85b62
Merge remote-tracking branch 'origin/main' into 16871-adapt-StatsSign…
IvanKavaldzhiev Jan 10, 2025
2ab240e
nit: resolve PR comment
IvanKavaldzhiev Jan 10, 2025
289fe7c
nit: fix PR comment
IvanKavaldzhiev Jan 10, 2025
32c9803
nit: resolve PR comment
IvanKavaldzhiev Jan 10, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -211,9 +211,7 @@ public synchronized CryptocurrencyDemoState copy() {
public void handleConsensusRound(
@NonNull final Round round,
@NonNull final PlatformStateModifier platformState,
@NonNull
final Consumer<List<ScopedSystemTransaction<StateSignatureTransaction>>>
stateSignatureTransactions) {
@NonNull final Consumer<ScopedSystemTransaction<StateSignatureTransaction>> stateSignatureTransactions) {
throwIfImmutable();
round.forEachEventTransaction((event, transaction) -> handleTransaction(event.getCreatorId(), transaction));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,7 @@ private HelloSwirldDemoState(final HelloSwirldDemoState sourceState) {
public synchronized void handleConsensusRound(
@NonNull final Round round,
@NonNull final PlatformStateModifier platformState,
@NonNull
final Consumer<List<ScopedSystemTransaction<StateSignatureTransaction>>>
stateSignatureTransactions) {
@NonNull final Consumer<ScopedSystemTransaction<StateSignatureTransaction>> stateSignatureTransactions) {
throwIfImmutable();
round.forEachTransaction(this::handleTransaction);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
import com.swirlds.platform.system.Round;
import com.swirlds.platform.system.SoftwareVersion;
import edu.umd.cs.findbugs.annotations.NonNull;
import java.util.List;
import java.util.function.Consumer;
import java.util.function.Function;

Expand Down Expand Up @@ -83,9 +82,7 @@ private StatsDemoState(final StatsDemoState sourceState) {
public void handleConsensusRound(
@NonNull final Round round,
@NonNull final PlatformStateModifier platformState,
@NonNull
final Consumer<List<ScopedSystemTransaction<StateSignatureTransaction>>>
stateSignatureTransactions) {}
@NonNull final Consumer<ScopedSystemTransaction<StateSignatureTransaction>> stateSignatureTransactions) {}

/**
* {@inheritDoc}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@
import java.text.ParseException;
import java.time.Duration;
import java.util.Iterator;
import java.util.List;
import java.util.Objects;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicReference;
Expand Down Expand Up @@ -222,9 +221,7 @@ public void init(
public void handleConsensusRound(
@NonNull final Round round,
@NonNull final PlatformStateModifier platformState,
@NonNull
final Consumer<List<ScopedSystemTransaction<StateSignatureTransaction>>>
stateSignatureTransactions) {
@NonNull final Consumer<ScopedSystemTransaction<StateSignatureTransaction>> stateSignatureTransactions) {
Objects.requireNonNull(round, "the round cannot be null");
Objects.requireNonNull(platformState, "the platform state cannot be null");
throwIfImmutable();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@
import java.nio.file.Files;
import java.nio.file.Path;
import java.time.Duration;
import java.util.List;
import java.util.Objects;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
Expand Down Expand Up @@ -246,9 +245,7 @@ private void applyTransactionToState(final @NonNull ConsensusTransaction transac
@Override
public void preHandle(
@NonNull final Event event,
@NonNull
final Consumer<List<ScopedSystemTransaction<StateSignatureTransaction>>>
stateSignatureTransactions) {
@NonNull final Consumer<ScopedSystemTransaction<StateSignatureTransaction>> stateSignatureTransactions) {
mustafauzunn marked this conversation as resolved.
Show resolved Hide resolved
event.forEachTransaction(transaction -> {
if (transaction.isSystem()) {
return;
Expand All @@ -272,9 +269,7 @@ public void preHandle(
public void handleConsensusRound(
final @NonNull Round round,
final @NonNull PlatformStateModifier platformState,
@NonNull
final Consumer<List<ScopedSystemTransaction<StateSignatureTransaction>>>
stateSignatureTransactions) {
@NonNull final Consumer<ScopedSystemTransaction<StateSignatureTransaction>> stateSignatureTransactions) {
mustafauzunn marked this conversation as resolved.
Show resolved Hide resolved
Objects.requireNonNull(round);
Objects.requireNonNull(platformState);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -243,9 +243,7 @@ <T extends SelfSerializable> void writeObjectByChildIndex(int index, List<T> lis
public void handleConsensusRound(
@NonNull final Round round,
@NonNull final PlatformStateModifier platformState,
@NonNull
final Consumer<List<ScopedSystemTransaction<StateSignatureTransaction>>>
stateSignatureTransactions) {
@NonNull final Consumer<ScopedSystemTransaction<StateSignatureTransaction>> stateSignatureTransactions) {
throwIfImmutable();
final Iterator<ConsensusEvent> eventIterator = round.iterator();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -285,9 +285,7 @@ public void init(
public void handleConsensusRound(
@NonNull final Round round,
@NonNull final PlatformStateModifier platformState,
@NonNull
final Consumer<List<ScopedSystemTransaction<StateSignatureTransaction>>>
stateSignatureTransactions) {
@NonNull final Consumer<ScopedSystemTransaction<StateSignatureTransaction>> stateSignatureTransactions) {
throwIfImmutable();
for (final Iterator<ConsensusEvent> eventIt = round.iterator(); eventIt.hasNext(); ) {
final ConsensusEvent event = eventIt.next();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1056,9 +1056,7 @@ protected void preHandleTransaction(final Transaction transaction) {
public synchronized void handleConsensusRound(
@NonNull final Round round,
@NonNull final PlatformStateModifier platformState,
@NonNull
final Consumer<List<ScopedSystemTransaction<StateSignatureTransaction>>>
stateSignatureTransactions) {
@NonNull final Consumer<ScopedSystemTransaction<StateSignatureTransaction>> stateSignatureTransactions) {
throwIfImmutable();
if (!initialized.get()) {
throw new IllegalStateException("handleConsensusRound() called before init()");
Expand Down Expand Up @@ -1662,9 +1660,7 @@ private static class ChildIndices {
@Override
public void preHandle(
@NonNull final Event event,
@NonNull
final Consumer<List<ScopedSystemTransaction<StateSignatureTransaction>>>
stateSignatureTransactions) {
@NonNull final Consumer<ScopedSystemTransaction<StateSignatureTransaction>> stateSignatureTransactions) {
event.forEachTransaction(this::preHandleTransaction);
}
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,29 @@
// SPDX-License-Identifier: Apache-2.0
/*
* Copyright (C) 2024 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.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

plugins { id("org.hiero.gradle.module.application") }

// Remove the following line to enable all 'javac' lint checks that we have turned on by default
// and then fix the reported issues.
tasks.withType<JavaCompile>().configureEach { options.compilerArgs.add("-Xlint:-cast") }

application.mainClass = "com.swirlds.demo.stats.signing.StatsSigningTestingToolMain"

testModuleInfo {
requires("org.assertj.core")
requires("org.junit.jupiter.api")
requires("org.mockito")
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2022-2024 Hedera Hashgraph, LLC
* Copyright (C) 2024 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,8 @@
import static com.swirlds.platform.test.fixtures.state.FakeMerkleStateLifecycles.FAKE_MERKLE_STATE_LIFECYCLES;
import static com.swirlds.platform.test.fixtures.state.FakeMerkleStateLifecycles.registerMerkleStateRootClassIds;

import com.hedera.hapi.platform.event.StateSignatureTransaction;
import com.hedera.pbj.runtime.io.buffer.Bytes;
import com.swirlds.common.constructable.ClassConstructorPair;
import com.swirlds.common.constructable.ConstructableRegistry;
import com.swirlds.common.constructable.ConstructableRegistryException;
Expand Down Expand Up @@ -313,4 +315,9 @@
public BasicSoftwareVersion getSoftwareVersion() {
return softwareVersion;
}

@Override
public Bytes encodeSystemTransaction(@NonNull StateSignatureTransaction transaction) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add final

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

return StateSignatureTransaction.PROTOBUF.toBytes(transaction);

Check warning on line 321 in platform-sdk/platform-apps/tests/StatsSigningTestingTool/src/main/java/com/swirlds/demo/stats/signing/StatsSigningTestingToolMain.java

View check run for this annotation

Codecov / codecov/patch

platform-sdk/platform-apps/tests/StatsSigningTestingTool/src/main/java/com/swirlds/demo/stats/signing/StatsSigningTestingToolMain.java#L321

Added line #L321 was not covered by tests
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2022-2024 Hedera Hashgraph, LLC
* Copyright (C) 2024 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 @@ -115,15 +115,19 @@ public synchronized StatsSigningTestingToolState copy() {
@Override
public void preHandle(
@NonNull final Event event,
@NonNull
final Consumer<List<ScopedSystemTransaction<StateSignatureTransaction>>>
stateSignatureTransactions) {
@NonNull final Consumer<ScopedSystemTransaction<StateSignatureTransaction>> stateSignatureTransactions) {
final SttTransactionPool sttTransactionPool = transactionPoolSupplier.get();
if (sttTransactionPool != null) {
event.forEachTransaction(transaction -> {
if (transaction.isSystem()) {
return;
}

if (areTransactionBytesSystemOnes((ConsensusTransaction) transaction)) {
stateSignatureTransactions.accept(
new ScopedSystemTransaction(event.getCreatorId(), event.getSoftwareVersion(), transaction));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can return after this, since the application should not do anything with the system transactions other than invoke the callback.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea. Fixed

}

final TransactionSignature transactionSignature =
sttTransactionPool.expandSignatures(transaction.getApplicationTransaction());
if (transactionSignature != null) {
Expand All @@ -141,17 +145,27 @@ public void preHandle(
public void handleConsensusRound(
@NonNull final Round round,
@NonNull final PlatformStateModifier platformState,
@NonNull
final Consumer<List<ScopedSystemTransaction<StateSignatureTransaction>>>
stateSignatureTransactions) {
@NonNull final Consumer<ScopedSystemTransaction<StateSignatureTransaction>> stateSignatureTransactions) {
throwIfImmutable();
round.forEachTransaction(this::handleTransaction);

round.forEachEventTransaction((event, transaction) -> {
final var transactionWithSystemBytes = handleTransaction(transaction);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not check if it is a system transaction here in the lambda before invoking handleTransaction?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated

if (transactionWithSystemBytes != null) {
stateSignatureTransactions.accept(new ScopedSystemTransaction(
event.getCreatorId(), event.getSoftwareVersion(), transactionWithSystemBytes));
}
});
}

private void handleTransaction(final ConsensusTransaction trans) {
private ConsensusTransaction handleTransaction(final ConsensusTransaction trans) {
if (trans.isSystem()) {
return;
return null;
}

if (areTransactionBytesSystemOnes(trans)) {
return trans;
}

final TransactionSignature s = trans.getMetadata();

if (s != null && validateSignature(s, trans) && s.getSignatureStatus() != VerificationStatus.VALID) {
Expand All @@ -177,6 +191,23 @@ private void handleTransaction(final ConsensusTransaction trans) {
runningSum += TransactionCodec.txId(trans.getApplicationTransaction());

maybeDelay();

return null;
}

/**
* Checks if the transaction bytes are system ones.
*
* @param transaction the consensus transaction to check
* @return true if the transaction bytes are system ones, false otherwise
*/
private boolean areTransactionBytesSystemOnes(final ConsensusTransaction transaction) {
// We have maximum allocation of 100 bytes for the transaction + 10 bytes for the preamble size in
// TransactionCodec
// + 64 bytes for the maximum public key + 64 bytes for the maximum signature size + 12 bytes for 3 integer
// bytes
final var maximumSignedEncodedTransactionSize = 100 + 10 + 64 + 64 + 12;
return transaction.getApplicationTransaction().length() > maximumSignedEncodedTransactionSize;
}

private void maybeDelay() {
Expand Down
Loading
Loading