feat(21868): Introduce configurable network topology #21927

kaldun-tech · 2025-10-29T22:23:42Z

Description:

Allow users to configure mesh and geographic topologies before adding nodes.

Related issue(s):

Fixes #21868

Notes for reviewer:

New:

MeshTopologyConfiguration record with builder methods
Network.withMeshTopology() to configure mesh topology
Network.withGeoMeshTopology() to configure geographic topology
TopologyConfigurationTest with 8 test cases

Modified:

Network.java - Added configuration methods
AbstractNetwork.java - Factory pattern implementation
MeshTopologyImpl.java - Configuration support

Design:

Pre-node configuration enforced (IllegalStateException if attempted after)
Factory pattern: Network creates topologies internally
Builder pattern: Immutable records with with...() methods
Fully backward compatible

Checklist

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

lfdt-bot · 2025-10-29T22:23:58Z

✅ Snyk checks have passed. No issues have been found so far.

Status	Scanner	Critical	High	Medium	Low	Total (0)
✅	Open Source Security	0	0	0	0	0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

Fixes hiero-ledger#21868 Allow users to configure mesh and geographic topologies before adding nodes. New: - MeshTopologyConfiguration record with builder methods - Network.withMeshTopology() to configure mesh topology - Network.withGeoMeshTopology() to configure geographic topology - TopologyConfigurationTest with 8 test cases Modified: - Network.java - Added configuration methods - AbstractNetwork.java - Factory pattern implementation - MeshTopologyImpl.java - Configuration support Design: - Pre-node configuration enforced (IllegalStateException if attempted after) - Factory pattern: Network creates topologies internally - Builder pattern: Immutable records with with...() methods - Fully backward compatible Usage: network.withMeshTopology( MeshTopologyConfiguration.DEFAULT .withAverageLatency(Duration.ofMillis(300)) .withJitter(Percentage.withPercentage(10)) ) .addNodes(4) .start(); Signed-off-by: kaldun-tech <[email protected]>

- Replace wildcard imports with explicit static imports in TopologyConfigurationTest - Add assertion messages to all JUnit and AssertJ assertions across test files - Remove unused private fields from AbstractNetwork Improves test diagnostics and removes dead code. Signed-off-by: kaldun-tech <[email protected]>

…changes Signed-off-by: kaldun-tech <[email protected]>

…s previously final and introducing a setter causes the warning to appear Signed-off-by: kaldun-tech <[email protected]>

kaldun-tech · 2025-10-31T17:44:48Z

The main issue I'm having here is the assignee check

netopyr · 2025-11-04T12:25:41Z

Hi @kaldun-tech! Sorry, I missed that your PR is ready for review. I set the assignee and the milestone. I will review your PR later, once I have determined the process. Can you please take a look at the conflict meanwhile?

Signed-off-by: Taras <[email protected]>

kaldun-tech · 2025-11-04T21:20:01Z

@netopyr Thanks Michael, the conflict seemed straightforward to fix. My change updated the name of the topology field to internalTopology to clean up Codacy warnings. The incoming change just added a networkConfiguration field.

netopyr

Thanks @kaldun-tech. Looks good so far. The only major change is to have a single additional method on Network.

netopyr · 2025-11-05T09:07:55Z

platform-sdk/consensus-otter-tests/src/testFixtures/java/org/hiero/otter/fixtures/Network.java

+     * @throws IllegalStateException if any nodes have already been added to the network
+     */
+    @NonNull
+    Network withMeshTopology(@NonNull MeshTopologyConfiguration configuration);


There should be only one method to configure the topology. We plan to add more topology types later, and it would be excessive to have a method for each. This means you have to define a common interface or abstract class for both configuration classes.

Please rename the method to topology(), returning void. This would make it consistent with the "record-style setters" we use elsewhere.

I agree creating this interface is sensible. It can be called TopologyConfiguration and these classes will implement that.

netopyr · 2025-11-05T09:09:34Z

...sus-otter-tests/src/testFixtures/java/org/hiero/otter/fixtures/internal/AbstractNetwork.java

-    private final Topology topology;
    private final boolean useRandomNodeIds;

+    private Topology internalTopology;


Do you think there will be another topology? If there is an internal topology, I immediately wonder if there is a second one that is not internal.

This was done to prevent Codacy warnings due to the name of the previously final variable being the same as the method. I tried to follow conventions observed in nearby classes. My personal habit would have been to name the methods as "getTopology setTopology"

I'll rename it "currentTopology" to be more clear that there is a single topology

netopyr · 2025-11-05T09:11:00Z

...sus-otter-tests/src/testFixtures/java/org/hiero/otter/fixtures/internal/AbstractNetwork.java

+    @Override
+    @NonNull
+    public Network withMeshTopology(@NonNull final MeshTopologyConfiguration configuration) {
+        throwIfInLifecycle(Lifecycle.RUNNING, "Cannot configure topology while the network is running.");


It would be sufficient to check if we are in Lifecycle.INIT. Once the network was started, we should not change the topology.

netopyr · 2025-11-05T09:22:14Z

...otter-tests/src/testIntegration/java/org/hiero/otter/fixtures/TopologyConfigurationTest.java

+ * before adding any nodes to the network using the new configuration objects
+ * and methods.
+ */
+class TopologyConfigurationTest {


This test does not cause harm, but we typically do not validate regular getters and setters. (Though, I would not be surprised if there are examples in our code base where we do exactly that. 😄 )

netopyr · 2025-11-05T09:24:59Z

platform-sdk/consensus-otter-tests/src/testFixtures/java/org/hiero/otter/fixtures/Network.java

+     * @throws IllegalStateException if any nodes have already been added to the network
+     */
+    @NonNull
+    Network withGeoMeshTopology(@NonNull GeographicLatencyConfiguration configuration);


GeographicLatencyConfiguration should probably be renamed to make it more evident that its sole purpose is to configure a GeoMeshTopology. Maybe GeoMeshTopologyConfiguration.

poulok · 2025-11-06T13:18:51Z

@kaldun-tech thank you for your interest and contribution to the project!

kaldun-tech · 2025-11-06T17:15:54Z

@poulok Thanks for the kind words! Planning to wrap up the PR today

kaldun-tech · 2025-11-07T01:49:44Z

I had trouble with the integration tests. It looks like the testIntegration gradle task depends on :consensus-otter-docker-app:copyDockerizedApp which is not present in the build in my fork. Is this already addressed in another issue?

netopyr · 2025-11-07T07:00:47Z

I had trouble with the integration tests. It looks like the testIntegration gradle task depends on :consensus-otter-docker-app:copyDockerizedApp which is not present in the build in my fork. Is this already addressed in another issue?

I do not recall ever seeing such an issue. The task copyDockerizedApp has been there for a long time, and I can also see it in the build script in your fork.

We had an issue, though, that one had to manually call assemble before running the integration tests. This was fixed recently. Could you please merge the latest changes from main and verify if the issue persists?

- Created TopologyConfiguration marker interface - Unified withMeshTopology() and withGeoMeshTopology() into single topology() method - Simplified lifecycle validation to use throwIfNotInLifecycle(Lifecycle.INIT) - Renamed internal field: internalTopology → currentTopology Addresses maintainer feedback on PR hiero-ledger#21927 Signed-off-by: kaldun-tech <[email protected]>

Extract GeoMeshTopologyConfiguration from inline configuration logic into a dedicated record class that implements TopologyConfiguration. This improves code organization and reusability by providing a standardized way to define topology configurations with validation and builder-style methods. Changes: - Create GeoMeshTopologyConfiguration record with validation - Introduce TopologyConfiguration marker interface - Update GeoMeshTopologyImpl to use extracted configuration - Refactor GeoMeshTopology to use configuration object - Add TopologyConfigurationTest for validation tests - Apply spotless formatting fixes across related files This enables easier topology customization and testing in network fixtures. Signed-off-by: kaldun-tech <[email protected]>

…opologyConfiguration Signed-off-by: kaldun-tech <[email protected]>

kaldun-tech · 2025-11-07T22:23:48Z

@netopyr @poulok I believe all concerns are addressed in the current state. Just running the integration tests again locally.

kaldun-tech force-pushed the 21868-Configurable-Topology branch 2 times, most recently from 7088108 to 52c7a22 Compare October 29, 2025 22:51

kaldun-tech force-pushed the 21868-Configurable-Topology branch from 52c7a22 to 426530e Compare October 30, 2025 00:12

kaldun-tech added 3 commits October 30, 2025 11:26

Corrected issues in spotless gradle checks by automatically applying …

da8a58b

…changes Signed-off-by: kaldun-tech <[email protected]>

Renamed topology in AbstractNetwork for Codacy warning. This field wa…

69b37b6

…s previously final and introducing a setter causes the warning to appear Signed-off-by: kaldun-tech <[email protected]>

kaldun-tech marked this pull request as ready for review October 31, 2025 17:42

kaldun-tech requested a review from a team as a code owner October 31, 2025 17:42

kaldun-tech requested a review from timo0 October 31, 2025 17:42

netopyr assigned kaldun-tech Nov 4, 2025

netopyr added this to the v0.68 milestone Nov 4, 2025

netopyr requested review from netopyr and poulok November 4, 2025 12:22

Merge branch 'main' into 21868-Configurable-Topology

6a50121

Signed-off-by: Taras <[email protected]>

netopyr reviewed Nov 5, 2025

View reviewed changes

kaldun-tech added 3 commits November 7, 2025 11:30

Removed GeographicLatencyConnfiguration which was renamed as GeoMeshT…

68d3495

…opologyConfiguration Signed-off-by: kaldun-tech <[email protected]>

feat(21868): Introduce configurable network topology #21927

Are you sure you want to change the base?

feat(21868): Introduce configurable network topology #21927

Conversation

kaldun-tech commented Oct 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lfdt-bot commented Oct 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Snyk checks have passed. No issues have been found so far.

Uh oh!

kaldun-tech commented Oct 31, 2025

Uh oh!

netopyr commented Nov 4, 2025

Uh oh!

kaldun-tech commented Nov 4, 2025

Uh oh!

netopyr left a comment

Choose a reason for hiding this comment

Uh oh!

netopyr Nov 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kaldun-tech Nov 5, 2025

Choose a reason for hiding this comment

Uh oh!

netopyr Nov 5, 2025

Choose a reason for hiding this comment

Uh oh!

kaldun-tech Nov 5, 2025

Choose a reason for hiding this comment

Uh oh!

kaldun-tech Nov 5, 2025

Choose a reason for hiding this comment

Uh oh!

netopyr Nov 5, 2025

Choose a reason for hiding this comment

Uh oh!

netopyr Nov 5, 2025

Choose a reason for hiding this comment

Uh oh!

netopyr Nov 5, 2025

Choose a reason for hiding this comment

Uh oh!

poulok commented Nov 6, 2025

Uh oh!

kaldun-tech commented Nov 6, 2025

Uh oh!

kaldun-tech commented Nov 7, 2025

Uh oh!

netopyr commented Nov 7, 2025

Uh oh!

kaldun-tech commented Nov 7, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

kaldun-tech commented Oct 29, 2025 •

edited

Loading

lfdt-bot commented Oct 29, 2025 •

edited

Loading

netopyr Nov 5, 2025 •

edited

Loading