Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
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
Expand Up @@ -320,4 +320,101 @@ public void Deserialize_WithMissingTypeI_ThrowsJsonException()
// Act & Assert
JsonSerializer.Deserialize<EffectResponse>(json, _options);
}

/// <summary>
/// Tests deserialization of a contract_credited effect with type_i=96.
/// Payload shape taken verbatim from a live Horizon mainnet response emitted by a
/// Soroban SAC transfer. Verifies that type_i=96 dispatches to
/// <see cref="ContractCreditedEffectResponse" /> and that every wire-format field is
/// bound to the expected property.
Comment on lines +330 to +331
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

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

The XML doc for this new test claims "every wire-format field" is verified, but the assertions only cover the subtype dispatch and a subset of fields (TypeId/Type/Contract/Amount/Asset*). Consider either asserting the other common fields (e.g., Id, PagingToken, Account, CreatedAt, Links) or tightening the doc comment to match what the test actually verifies.

Suggested change
/// <see cref="ContractCreditedEffectResponse" /> and that every wire-format field is
/// bound to the expected property.
/// <see cref="ContractCreditedEffectResponse" /> and that the contract, amount, and
/// asset-related fields asserted below are bound to the expected properties.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added assertions.

/// </summary>
[TestMethod]
public void Deserialize_WithContractCreditedEffectTypeI96_ReturnsContractCreditedEffectResponse()
{
// Arrange - real Horizon mainnet payload (https://horizon.stellar.org/effects, operation 266896217031864321-13).
const string json =
"""
{
"_links": {
"operation": {"href": "https://horizon.stellar.org/operations/266896217031864321"},
"succeeds": {"href": "https://horizon.stellar.org/effects?order=desc&cursor=266896217031864321-13"},
"precedes": {"href": "https://horizon.stellar.org/effects?order=asc&cursor=266896217031864321-13"}
},
"id": "0266896217031864321-0000000013",
"paging_token": "266896217031864321-13",
"account": "GBKDPWX3BY5PWVWY6CJVY57CFD3I54VXJZPEZTUGTBFDNBGMCSWNXTEA",
"type": "contract_credited",
"type_i": 96,
"created_at": "2026-04-16T06:43:24Z",
"asset_type": "native",
"contract": "CB2TCA23Z3CLTXSVGJB2PAC3MTRNX6NPFETRRDDJEAVBJMAZIUPIKBZL",
"amount": "1.0152349"
}
""";

// Act
var result = JsonSerializer.Deserialize<EffectResponse>(json, _options);

// Assert - type dispatch + field binding
Assert.IsNotNull(result);
Assert.IsInstanceOfType(result, typeof(ContractCreditedEffectResponse));

var credited = (ContractCreditedEffectResponse)result;
Assert.AreEqual(96, credited.TypeId);
Assert.AreEqual("contract_credited", credited.Type);
Assert.AreEqual("CB2TCA23Z3CLTXSVGJB2PAC3MTRNX6NPFETRRDDJEAVBJMAZIUPIKBZL", credited.Contract);
Assert.AreEqual("1.0152349", credited.Amount);
Assert.AreEqual("native", credited.AssetType);
Assert.IsNull(credited.AssetCode);
Assert.IsNull(credited.AssetIssuer);
}

/// <summary>
/// Tests deserialization of a contract_debited effect with type_i=97.
/// Payload shape constructed from the same wire format Horizon emits for the symmetric
/// SAC transfer debit, with a credit_alphanum4 asset to exercise the non-native branch
/// of <see cref="ContractDebitedEffectResponse.Asset" />.
/// </summary>
Comment on lines +395 to +400
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

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

This test's summary says the credit_alphanum4 payload is used to exercise the non-native branch of ContractDebitedEffectResponse.Asset, but the test never accesses or asserts the computed Asset property. If the intent is to cover that branch, add an assertion against debited.Asset (type/code/issuer) or update the comment to avoid implying coverage that isn't present.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added the assertion.

[TestMethod]
public void Deserialize_WithContractDebitedEffectTypeI97_ReturnsContractDebitedEffectResponse()
{
// Arrange - wire shape matches Horizon's emitted contract_debited effect.
const string json =
"""
{
"_links": {
"operation": {"href": "https://horizon.stellar.org/operations/266896217031864321"},
"succeeds": {"href": "https://horizon.stellar.org/effects?order=desc&cursor=266896217031864321-14"},
"precedes": {"href": "https://horizon.stellar.org/effects?order=asc&cursor=266896217031864321-14"}
},
"id": "0266896217031864321-0000000014",
"paging_token": "266896217031864321-14",
"account": "GBKDPWX3BY5PWVWY6CJVY57CFD3I54VXJZPEZTUGTBFDNBGMCSWNXTEA",
"type": "contract_debited",
"type_i": 97,
"created_at": "2026-04-16T06:43:24Z",
"asset_type": "credit_alphanum4",
"asset_code": "USDC",
"asset_issuer": "GA5ZSEJYB37JRC5AVCIA5MOP4RHTM335X2KGX3IHOJAPP5RE34K4KZVN",
"contract": "CB2TCA23Z3CLTXSVGJB2PAC3MTRNX6NPFETRRDDJEAVBJMAZIUPIKBZL",
"amount": "42.0000000"
}
""";

// Act
var result = JsonSerializer.Deserialize<EffectResponse>(json, _options);

// Assert - type dispatch + field binding (including nullable asset_code / asset_issuer)
Assert.IsNotNull(result);
Assert.IsInstanceOfType(result, typeof(ContractDebitedEffectResponse));

var debited = (ContractDebitedEffectResponse)result;
Assert.AreEqual(97, debited.TypeId);
Assert.AreEqual("contract_debited", debited.Type);
Assert.AreEqual("CB2TCA23Z3CLTXSVGJB2PAC3MTRNX6NPFETRRDDJEAVBJMAZIUPIKBZL", debited.Contract);
Assert.AreEqual("42.0000000", debited.Amount);
Assert.AreEqual("credit_alphanum4", debited.AssetType);
Assert.AreEqual("USDC", debited.AssetCode);
Assert.AreEqual("GA5ZSEJYB37JRC5AVCIA5MOP4RHTM335X2KGX3IHOJAPP5RE34K4KZVN", debited.AssetIssuer);
}
}
7 changes: 5 additions & 2 deletions StellarDotnetSdk/Converters/EffectResponseJsonConverter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ namespace StellarDotnetSdk.Converters;
/// 50-52 = Claimable balance,
/// 60-74 = Sponsorship effects
/// 80 = Clawback
/// 90-95 = Liquidity pool effects.
/// 90-95 = Liquidity pool effects
/// 96-97 = Soroban SAC contract transfer effects.
/// Performance: Parses JSON once into JsonDocument, then deserializes from JsonElement
/// to avoid double-parsing overhead.
/// <br />
Expand Down Expand Up @@ -119,9 +120,11 @@ public override bool CanConvert(Type typeToConvert)
93 => root.Deserialize<LiquidityPoolCreatedEffectResponse>(options),
94 => root.Deserialize<LiquidityPoolRemovedEffectResponse>(options),
95 => root.Deserialize<LiquidityPoolRevokedEffectResponse>(options),
96 => root.Deserialize<ContractCreditedEffectResponse>(options),
97 => root.Deserialize<ContractDebitedEffectResponse>(options),
Comment on lines +123 to +124
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

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

The PR description/checkboxes state that documentation changes were required and already updated, but this PR only changes the converter and its tests (no docs/README updates). Either include the intended documentation updates or adjust the PR description/checkboxes to avoid a mismatch for reviewers and release notes.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated.

_ => throw new JsonException(
$"Unknown effect type_i: {type}. " +
$"Expected value in ranges 0-7, 10-12, 20-26, 30-33, 40-43, 50-52, 60-74, 80, or 90-95. " +
$"Expected value in ranges 0-7, 10-12, 20-26, 30-33, 40-43, 50-52, 60-74, 80, 90-95, or 96-97. " +
$"This may indicate an API version mismatch or a new effect type. Check if your SDK version supports this effect type."
),
};
Expand Down
Loading