feat: Remove `TssEncryptionKeyTransaction`'s from state #16772

petreze · 2024-11-25T14:19:21Z

Description:
This PR aims to clear any TssEncryptionKeyTransactionBody from state if the node id mapped to the transaction body is not present in any of the active rosters or a candidate roster. This cleanup is added on restart() of RostersSchema, so that the state changes are present in the block stream.

Related issue(s):

Fixes #16570

Notes for reviewer:

Checklist

Documented (Code comments, README, etc.)
Tested (unit, integration, etc.)

Signed-off-by: Derek Riley <[email protected]>

…ice-EncryptionKeyState

Signed-off-by: Derek Riley <[email protected]>

# Conflicts: # hedera-node/hedera-app/src/main/java/com/hedera/node/app/Hedera.java # hedera-node/hedera-app/src/main/java/com/hedera/node/app/HederaInjectionComponent.java # hedera-node/hedera-app/src/main/java/com/hedera/node/app/ServicesMain.java # hedera-node/hedera-app/src/main/java/com/hedera/node/app/tss/TssBaseService.java # hedera-node/hedera-app/src/main/java/com/hedera/node/app/tss/stores/ReadableTssStore.java # hedera-node/hedera-app/src/main/java/com/hedera/node/app/tss/stores/ReadableTssStoreImpl.java # hedera-node/hedera-app/src/main/java/com/hedera/node/app/workflows/handle/HandleWorkflow.java # hedera-node/hedera-app/src/test/java/com/hedera/node/app/components/IngestComponentTest.java # hedera-node/hedera-app/src/testFixtures/java/com/hedera/node/app/fixtures/state/FakePlatform.java # hedera-node/hedera-config/src/main/java/com/hedera/node/config/data/NetworkAdminConfig.java # hedera-node/test-clients/src/main/java/com/hedera/services/bdd/junit/hedera/embedded/fakes/FakeTssBaseService.java # platform-sdk/swirlds-platform-core/src/main/java/com/swirlds/platform/system/Platform.java # platform-sdk/swirlds-platform-core/src/main/java/module-info.java

Signed-off-by: Petar Tonev <[email protected]>

# Conflicts: # platform-sdk/swirlds-platform-core/src/main/java/com/swirlds/platform/crypto/CryptoStatic.java

Signed-off-by: Petar Tonev <[email protected]>

# Conflicts: # hapi/hedera-protobufs/block/stream/output/state_changes.proto # hedera-node/hedera-app/src/main/java/com/hedera/node/app/blocks/impl/KVStateChangeListener.java # hedera-node/hedera-app/src/main/java/com/hedera/node/app/workflows/handle/HandleWorkflow.java # hedera-node/hedera-app/src/test/java/com/hedera/node/app/components/IngestComponentTest.java # hedera-node/test-clients/src/main/java/com/hedera/services/bdd/junit/support/validators/block/StateChangesValidator.java

…rate-tssEncryptionKey

Signed-off-by: Derek Riley <[email protected]>

…ice-EncryptionKeyState

Signed-off-by: Derek Riley <[email protected]>

Signed-off-by: Petar Tonev <[email protected]>

Signed-off-by: Derek Riley <[email protected]>

… into local-test # Conflicts: # hedera-node/hedera-app/src/main/java/module-info.java

This reverts commit b5d989b.

This reverts commit 2355cc0.

This reverts commit b1b42f1.

This reverts commit d78b884.

Signed-off-by: Petar Tonev <[email protected]>

jsync-swirlds

The use of a transaction body as a state value seems a bit odd and slightly concerning; perhaps we can simplify that?
Also a few small documentation items.

jsync-swirlds · 2024-11-27T16:15:20Z

hapi/hedera-protobufs/block/stream/output/state_changes.proto

@@ -384,6 +385,11 @@ enum StateIdentifier {
     */
    STATE_ID_SCHEDULED_USAGES = 34;

+    /**
+    * A state identifier for TSS Encryption Key


nit: Indentation is off.

jsync-swirlds · 2024-11-27T16:18:57Z

hapi/hedera-protobufs/block/stream/output/state_changes.proto

+        /**
+         * A TSS Encryption Key Transaction Body value.
+         */
+        com.hedera.hapi.services.auxiliary.tss.TssEncryptionKeyTransactionBody tss_encryption_key_value = 20;


This seems very odd and concerning. Are we truly storing full transaction bodies in state? If we are, I would recommend changing that, particularly as this message only has one field (of bytes).
We could more clearly store the bytes as a value in state with the same "key" used (is it Node ID?) to avoid the added wrapper message and also reduce potential confusion.

jsync-swirlds · 2024-11-27T16:24:50Z

hapi/hedera-protobufs/services/auxiliary/tss/tss_encryption_key.proto

+ */
+message TssEncryptionKeyTransactionBody {
+  /**
+   * The raw bytes of the public TSS encryption key of the node sending the transaction.


Suggested change

* The raw bytes of the public TSS encryption key of the node sending the transaction.

* An array of public TSS key bytes.<br/>

* This is the raw bytes of the public TSS encryption key for

* the node sending the transaction.

jsync-swirlds · 2024-11-27T16:25:17Z

hapi/hedera-protobufs/services/auxiliary/tss/tss_encryption_key.proto

+option java_multiple_files = true;
+
+/**
+ * A transaction body for sending the public TSS encryption key.


This is missing requirements, detail, and block stream effects.

jsync-swirlds · 2024-11-27T16:33:06Z

...-node/hedera-app/src/main/java/com/hedera/node/app/tss/handlers/TssEncryptionKeyHandler.java

+        final var tssStore = context.storeFactory().writableStore(WritableTssStore.class);
+        final var nodeEntityNumber =
+                EntityNumber.newBuilder().number(context.creatorInfo().nodeId()).build();
+        tssStore.put(nodeEntityNumber, op);


Instead of storing the entire transaction body, could we just store the bytes of the encryption key (which is the only field in the body)?
That would reduce state size and also make the block stream a little smaller.

mxtartaglia-sl · 2024-11-28T18:18:30Z

I want to note that the serialization mechanism for the keys is being reworked based on a decision to be compatible with Ethereum's format. While we work on the necessary modifications, the tss-library API will continue supporting the old formats. Still, the production-ready version will not be compatible with the uploaded files in this pr.
Can the files still be changed once the modification is complete? Or should we hold off on this PR until that time?

petreze

Thanks for the notice Maxi, I will put that on hold and mark it as a draft until finished. There are also other things for clarification until ready for a review

derektriley and others added 15 commits November 13, 2024 12:50

Add TssEncryptionKeyTransaction and map to state

d5eba98

Signed-off-by: Derek Riley <[email protected]>

More changes for TSS Encryption Key in state

6cf8ee7

Signed-off-by: Derek Riley <[email protected]>

More wip on TssEncryptionKeyHandler etc

891cfa1

Signed-off-by: Derek Riley <[email protected]>

WritableTssStore methods for TssEncryptionKey

3e0e469

Signed-off-by: Derek Riley <[email protected]>

wip

556ca36

Signed-off-by: Derek Riley <[email protected]>

Added TSS Encryption Key generation in EnhancedKeyStoreLoader

9acf000

Signed-off-by: Derek Riley <[email protected]>

Merge branch '14542-generate-tssEncryptionKey' into 16555-TssBaseServ…

50cf046

…ice-EncryptionKeyState

Implement TssEncryptionKey submission

889c8e5

Signed-off-by: Derek Riley <[email protected]>

Wip

99ef97c

Signed-off-by: Derek Riley <[email protected]>

wip

6fe3072

Signed-off-by: Derek Riley <[email protected]>

wip

f6bab2f

Signed-off-by: Derek Riley <[email protected]>

fixes after merge conflicts

b3de631

Signed-off-by: Petar Tonev <[email protected]>

test calls in unit tests

8bc925d

Signed-off-by: Petar Tonev <[email protected]>

Merge branch 'develop' into new-test

79f0c61

# Conflicts: # platform-sdk/swirlds-platform-core/src/main/java/com/swirlds/platform/crypto/CryptoStatic.java

petreze added this to the v0.58 milestone Nov 25, 2024

petreze self-assigned this Nov 25, 2024

petreze mentioned this pull request Nov 25, 2024

feat: Cleanup TssEncryptionKeyTransaction's from state #16690

Closed

2 tasks

petreze and others added 12 commits November 25, 2024 16:35

fix dependency

5dd4f75

Signed-off-by: Petar Tonev <[email protected]>

fix dependency

9bd8514

Signed-off-by: Petar Tonev <[email protected]>

Merge remote-tracking branch 'remotes/origin/develop' into 14542-gene…

bd19e1a

…rate-tssEncryptionKey

Added test resources, cleanup gradle

79edcb1

Signed-off-by: Derek Riley <[email protected]>

Update KeysAndCerts for deterministic generation

da91b85

Signed-off-by: Derek Riley <[email protected]>

wip

59cd3bc

Signed-off-by: Derek Riley <[email protected]>

module info and codacy

1d5bd12

Signed-off-by: Derek Riley <[email protected]>

Added another test case

186b839

Signed-off-by: Derek Riley <[email protected]>

Merge branch '14542-generate-tssEncryptionKey' into 16555-TssBaseServ…

ec81f70

…ice-EncryptionKeyState

spotless cleanup

7fd7891

Signed-off-by: Derek Riley <[email protected]>

more cleanup

04011eb

Signed-off-by: Derek Riley <[email protected]>

petreze and others added 10 commits November 26, 2024 16:08

remove suite

b6d3b16

Signed-off-by: Petar Tonev <[email protected]>

Update gradle build and module-info

3d7c771

Signed-off-by: Derek Riley <[email protected]>

Merge branch 'develop' into 16570-remove-tss-encryption-txs

82c021c

Merge remote-tracking branch 'origin/14542-generate-tssEncryptionKey'…

597008c

… into local-test # Conflicts: # hedera-node/hedera-app/src/main/java/module-info.java

Revert "remove suite"

6fcea11

This reverts commit b5d989b.

Revert "remove suite"

d3db8f4

This reverts commit 2355cc0.

Revert "spotless"

f87c45b

This reverts commit b1b42f1.

Revert "revert some"

576d2a1

This reverts commit d78b884.

Merge branch 'local-test' into 16570-remove-tss-encryption-txs

5d9637e

add migration order to tss base service

2d5e4db

Signed-off-by: Petar Tonev <[email protected]>

petreze marked this pull request as ready for review November 27, 2024 15:19

petreze requested review from a team and jsync-swirlds as code owners November 27, 2024 15:19

petreze requested review from lpetrovic05, hendrikebbers, dalvizu, jeromy-cannon, mhess-swl and artemananiev November 27, 2024 15:19

jsync-swirlds reviewed Nov 27, 2024

View reviewed changes

petreze commented Nov 29, 2024

View reviewed changes

petreze marked this pull request as draft November 29, 2024 10:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Remove `TssEncryptionKeyTransaction`'s from state #16772

feat: Remove `TssEncryptionKeyTransaction`'s from state #16772

petreze commented Nov 25, 2024 •

edited

Loading

jsync-swirlds left a comment

jsync-swirlds Nov 27, 2024

jsync-swirlds Nov 27, 2024

jsync-swirlds Nov 27, 2024

jsync-swirlds Nov 27, 2024

jsync-swirlds Nov 27, 2024

mxtartaglia-sl commented Nov 28, 2024

petreze left a comment

-   * The raw bytes of the public TSS encryption key of the node sending the transaction.
+   * An array of public TSS key bytes.<br/>
+   * This is the raw bytes of the public TSS encryption key for
+   * the node sending the transaction.

feat: Remove TssEncryptionKeyTransaction's from state #16772

Are you sure you want to change the base?

feat: Remove TssEncryptionKeyTransaction's from state #16772

Conversation

petreze commented Nov 25, 2024 • edited Loading

jsync-swirlds left a comment

Choose a reason for hiding this comment

jsync-swirlds Nov 27, 2024

Choose a reason for hiding this comment

jsync-swirlds Nov 27, 2024

Choose a reason for hiding this comment

jsync-swirlds Nov 27, 2024

Choose a reason for hiding this comment

jsync-swirlds Nov 27, 2024

Choose a reason for hiding this comment

jsync-swirlds Nov 27, 2024

Choose a reason for hiding this comment

mxtartaglia-sl commented Nov 28, 2024

petreze left a comment

Choose a reason for hiding this comment

feat: Remove `TssEncryptionKeyTransaction`'s from state #16772

feat: Remove `TssEncryptionKeyTransaction`'s from state #16772

petreze commented Nov 25, 2024 •

edited

Loading