Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
@@ -0,0 +1,166 @@
package com.stablecoin.payments.custody.domain.model;
Comment thread
coderabbitai[bot] marked this conversation as resolved.

import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;

import java.util.ArrayList;
import java.util.List;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;

@DisplayName("ChainConfig")
class ChainConfigTest {

private static final ChainId BASE = new ChainId("base");
private static final List<String> RPC_ENDPOINTS = List.of("https://rpc.base.org", "https://rpc2.base.org");

@Nested
@DisplayName("Valid Construction")
class ValidConstruction {

@Test
@DisplayName("creates chain config with all fields")
void createsChainConfig() {
var result = new ChainConfig(
BASE, 12, 2, "ETH", RPC_ENDPOINTS, "https://basescan.org"
);

var expected = new ChainConfig(
BASE, 12, 2, "ETH", RPC_ENDPOINTS, "https://basescan.org"
);
assertThat(result).usingRecursiveComparison().isEqualTo(expected);
}
Comment on lines +25 to +34

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Tautological test — comparing identically-constructed objects.

Same issue as TransferParticipantTest. Assert on individual accessors to verify field population.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@blockchain-custody/blockchain-custody/src/test/java/com/stablecoin/payments/custody/domain/model/ChainConfigTest.java`
around lines 26 - 35, The test creates two identical ChainConfig instances and
compares them tautologically; update the createsChainConfig test to assert each
field via ChainConfig's accessors instead of comparing two constructed objects.
Locate the createsChainConfig method in ChainConfigTest and replace the
recursive comparison with assertions that verify the values passed to the
ChainConfig constructor (e.g., the chain identifier constant BASE, numeric
values 12 and 2, the native symbol "ETH", RPC_ENDPOINTS, and the explorer URL
"https://basescan.org") by calling the corresponding getters on the result
object (the ChainConfig instance) to ensure each field is populated correctly.


@Test
@DisplayName("accepts zero minConfirmations")
void acceptsZeroConfirmations() {
var config = new ChainConfig(
BASE, 0, 2, "ETH", RPC_ENDPOINTS, "https://basescan.org"
);

assertThat(config.minConfirmations()).isZero();
}

@Test
@DisplayName("accepts zero avgFinalitySeconds")
void acceptsZeroFinality() {
var config = new ChainConfig(
BASE, 12, 0, "ETH", RPC_ENDPOINTS, "https://basescan.org"
);

assertThat(config.avgFinalitySeconds()).isZero();
}

@Test
@DisplayName("accepts null explorerUrl")
void acceptsNullExplorerUrl() {
var config = new ChainConfig(
BASE, 12, 2, "ETH", RPC_ENDPOINTS, null
);

assertThat(config.explorerUrl()).isNull();
}

@Test
@DisplayName("accepts single RPC endpoint")
void acceptsSingleEndpoint() {
var config = new ChainConfig(
BASE, 12, 2, "ETH", List.of("https://rpc.base.org"), null
);

assertThat(config.rpcEndpoints()).hasSize(1);
}
}

@Nested
@DisplayName("Validation")
class Validation {

@Test
@DisplayName("rejects null chainId")
void rejectsNullChainId() {
assertThatThrownBy(() -> new ChainConfig(
null, 12, 2, "ETH", RPC_ENDPOINTS, null
))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Chain ID is required");
}

@Test
@DisplayName("rejects negative minConfirmations")
void rejectsNegativeConfirmations() {
assertThatThrownBy(() -> new ChainConfig(
BASE, -1, 2, "ETH", RPC_ENDPOINTS, null
))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Min confirmations must be non-negative");
}

@Test
@DisplayName("rejects negative avgFinalitySeconds")
void rejectsNegativeFinality() {
assertThatThrownBy(() -> new ChainConfig(
BASE, 12, -1, "ETH", RPC_ENDPOINTS, null
))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Avg finality seconds must be non-negative");
}

@Test
@DisplayName("rejects null nativeToken")
void rejectsNullNativeToken() {
assertThatThrownBy(() -> new ChainConfig(
BASE, 12, 2, null, RPC_ENDPOINTS, null
))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Native token is required");
}

@Test
@DisplayName("rejects blank nativeToken")
void rejectsBlankNativeToken() {
assertThatThrownBy(() -> new ChainConfig(
BASE, 12, 2, " ", RPC_ENDPOINTS, null
))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Native token is required");
}

@Test
@DisplayName("rejects null rpcEndpoints")
void rejectsNullEndpoints() {
assertThatThrownBy(() -> new ChainConfig(
BASE, 12, 2, "ETH", null, null
))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("At least one RPC endpoint is required");
}

@Test
@DisplayName("rejects empty rpcEndpoints")
void rejectsEmptyEndpoints() {
assertThatThrownBy(() -> new ChainConfig(
BASE, 12, 2, "ETH", List.of(), null
))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("At least one RPC endpoint is required");
}
}

@Nested
@DisplayName("Defensive Copy")
class DefensiveCopy {

@Test
@DisplayName("rpcEndpoints are defensively copied")
void endpointsDefensivelyCopied() {
var mutableList = new ArrayList<>(List.of("https://rpc.base.org"));
var config = new ChainConfig(BASE, 12, 2, "ETH", mutableList, null);

assertThatThrownBy(() -> config.rpcEndpoints().add("https://evil.com"))
.isInstanceOf(UnsupportedOperationException.class);
}
Comment on lines +156 to +164

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Defensive copy test is incomplete.

This only verifies the returned list is unmodifiable. A proper defensive copy test should also verify that mutating the input list after construction doesn't affect the internal state.

Suggested enhancement
         `@Test`
-        `@DisplayName`("rpcEndpoints are defensively copied")
-        void endpointsDefensivelyCopied() {
+        `@DisplayName`("rpcEndpoints are defensively copied on input")
+        void inputMutationDoesNotAffectConfig() {
             var mutableList = new ArrayList<>(List.of("https://rpc.base.org"));
             var config = new ChainConfig(BASE, 12, 2, "ETH", mutableList, null);
+            
+            mutableList.add("https://malicious.com");

-            assertThatThrownBy(() -> config.rpcEndpoints().add("https://evil.com"))
-                    .isInstanceOf(UnsupportedOperationException.class);
+            assertThat(config.rpcEndpoints()).containsExactly("https://rpc.base.org");
         }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
@Test
@DisplayName("rpcEndpoints are defensively copied")
void endpointsDefensivelyCopied() {
var mutableList = new ArrayList<>(List.of("https://rpc.base.org"));
var config = new ChainConfig(BASE, 12, 2, "ETH", mutableList, null);
assertThatThrownBy(() -> config.rpcEndpoints().add("https://evil.com"))
.isInstanceOf(UnsupportedOperationException.class);
}
`@Test`
`@DisplayName`("rpcEndpoints are defensively copied on input")
void inputMutationDoesNotAffectConfig() {
var mutableList = new ArrayList<>(List.of("https://rpc.base.org"));
var config = new ChainConfig(BASE, 12, 2, "ETH", mutableList, null);
mutableList.add("https://malicious.com");
assertThat(config.rpcEndpoints()).containsExactly("https://rpc.base.org");
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@blockchain-custody/blockchain-custody/src/test/java/com/stablecoin/payments/custody/domain/model/ChainConfigTest.java`
around lines 157 - 165, The test endpointsDefensivelyCopied currently only
asserts the returned list from config.rpcEndpoints() is unmodifiable; extend it
to also verify the constructor defensively copied the input by mutating the
original mutableList after creating new ChainConfig(BASE, 12, 2, "ETH",
mutableList, null) (e.g., add/remove an element from mutableList) and then
asserting config.rpcEndpoints() still contains only the original RPC and was not
affected by the mutation; keep assertions using config.rpcEndpoints() and the
same test method.

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
package com.stablecoin.payments.custody.domain.model;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Fix Spotless formatting violations.

Pipeline failed with spotlessJavaCheck. Run:

./gradlew spotlessApply
🧰 Tools
🪛 GitHub Actions: CI

[error] 1-1: spotlessJavaCheck failed. Formatting violations detected in src/test/java/com/stablecoin/payments/custody/domain/model/ChainIdTest.java. Run './gradlew spotlessApply' to fix code style issues.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@blockchain-custody/blockchain-custody/src/test/java/com/stablecoin/payments/custody/domain/model/ChainIdTest.java`
at line 1, The file ChainIdTest has Spotless formatting violations; run the
formatter and apply fixes by running ./gradlew spotlessApply (or invoke your
IDE's Java formatter) and then reformat the ChainIdTest class and any affected
files so they comply with spotless rules; ensure imports, spacing, and line
endings in the ChainIdTest class (and surrounding domain.model classes if
flagged) are corrected before committing.


import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;

@DisplayName("ChainId")
class ChainIdTest {

@Nested
@DisplayName("Valid Chains")
class ValidChains {

@Test
@DisplayName("accepts 'ethereum'")
void acceptsEthereum() {
var chainId = new ChainId("ethereum");
assertThat(chainId.value()).isEqualTo("ethereum");
}

@Test
@DisplayName("accepts 'solana'")
void acceptsSolana() {
var chainId = new ChainId("solana");
assertThat(chainId.value()).isEqualTo("solana");
}

@Test
@DisplayName("accepts 'stellar'")
void acceptsStellar() {
var chainId = new ChainId("stellar");
assertThat(chainId.value()).isEqualTo("stellar");
}

@Test
@DisplayName("accepts 'tron'")
void acceptsTron() {
var chainId = new ChainId("tron");
assertThat(chainId.value()).isEqualTo("tron");
}

@Test
@DisplayName("accepts 'base'")
void acceptsBase() {
var chainId = new ChainId("base");
assertThat(chainId.value()).isEqualTo("base");
}

@Test
@DisplayName("accepts 'polygon'")
void acceptsPolygon() {
var chainId = new ChainId("polygon");
assertThat(chainId.value()).isEqualTo("polygon");
}

@Test
@DisplayName("accepts 'avalanche'")
void acceptsAvalanche() {
var chainId = new ChainId("avalanche");
assertThat(chainId.value()).isEqualTo("avalanche");
}
Comment on lines +19 to +66

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Consider parameterized test for valid chains.

Seven near-identical tests could be consolidated:

Suggested refactor
`@ParameterizedTest`
`@ValueSource`(strings = {"ethereum", "solana", "stellar", "tron", "base", "polygon", "avalanche"})
`@DisplayName`("accepts supported chain")
void acceptsSupportedChain(String chain) {
    var chainId = new ChainId(chain);
    assertThat(chainId.value()).isEqualTo(chain);
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@blockchain-custody/blockchain-custody/src/test/java/com/stablecoin/payments/custody/domain/model/ChainIdTest.java`
around lines 20 - 67, The seven near-identical tests (acceptsEthereum,
acceptsSolana, acceptsStellar, acceptsTron, acceptsBase, acceptsPolygon,
acceptsAvalanche) should be consolidated into a parameterized test to reduce
duplication: replace these individual `@Test` methods with a single
`@ParameterizedTest` using `@ValueSource`(strings =
{"ethereum","solana","stellar","tron","base","polygon","avalanche"}) and a
single test method (e.g., acceptsSupportedChain) that constructs new
ChainId(chain) and asserts chainId.value().isEqualTo(chain); keep the
DisplayName to describe supported chains.

}

@Nested
@DisplayName("Invalid Values")
class InvalidValues {

@Test
@DisplayName("rejects null value")
void rejectsNull() {
assertThatThrownBy(() -> new ChainId(null))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Chain ID value is required");
}

@ParameterizedTest
@ValueSource(strings = {"", " ", "\t"})
@DisplayName("rejects blank values")
void rejectsBlank(String value) {
assertThatThrownBy(() -> new ChainId(value))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Chain ID value is required");
}

@ParameterizedTest
@ValueSource(strings = {"bitcoin", "cardano", "ripple", "ETHEREUM", "Base", "SOLANA"})
@DisplayName("rejects unsupported chains")
void rejectsUnsupported(String value) {
assertThatThrownBy(() -> new ChainId(value))
.isInstanceOf(IllegalArgumentException.class)
.hasMessageContaining("Unsupported chain");
}
}

@Nested
@DisplayName("Equality")
class Equality {

@Test
@DisplayName("two ChainId with same value are equal")
void equalWhenSameValue() {
var a = new ChainId("base");
var b = new ChainId("base");

assertThat(a).isEqualTo(b);
assertThat(a.hashCode()).isEqualTo(b.hashCode());
}

@Test
@DisplayName("two ChainId with different values are not equal")
void notEqualWhenDifferentValue() {
var a = new ChainId("base");
var b = new ChainId("ethereum");

assertThat(a).isNotEqualTo(b);
}
}
}
Loading