diff --git a/.ci-config/docker-compose.ci.yml b/.ci-config/docker-compose.ci.yml index 6d483cf7..1c2da9d4 100644 --- a/.ci-config/docker-compose.ci.yml +++ b/.ci-config/docker-compose.ci.yml @@ -7,8 +7,10 @@ services: - "5005:5005" - "5006:5006" - "6006:6006" - # credential-protected admin ws port, see [port_ws_admin_auth] in rippled.cfg - - "6007:6007" + # credential-protected admin ws port, see [port_ws_admin_auth] in rippled.cfg. + # Loopback-only: rippled.cfg carries these credentials in plain text, so the port + # must not be reachable from other hosts. Tests connect over localhost. + - "127.0.0.1:6007:6007" volumes: - ./rippled.cfg:/config/rippled.cfg:ro - ./validators.txt:/config/validators.txt:ro diff --git a/CHANGES.md b/CHANGES.md index d1e22876..cbebaa68 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -34,6 +34,13 @@ * `TestIConnectionStates` (7 tests) pointed at the public testnet and devnet. Nothing in them is specific to a public network — every assertion is about the client's own state machine — so they now run against the local node. The bogus-hostname case that tested reconnect exhaustion used a DNS lookup; it now uses a closed loopback port, which refuses immediately and involves no resolver. Fixed `Task.Delay` sleeps were the other half of the flakiness (one of these tests failed a full run and passed on retry) and are replaced by waiting for the expected state with a timeout: the class went from ~40 s of sleeping to sub-second assertions * the x402 live t54 interop tests need the public testnet faucet *and* a hosted third-party facilitator. They are now `[TestCategory("Live")]` and excluded from CI (`--filter "TestI&TestCategory!=Live"`), leaving the six hermetic x402 E2E tests in the run. Invoke them deliberately with `--filter "TestCategory=Live"` +* Review pass (PR [#68](https://github.com/StaticBit-io/XrplCSharp/pull/68)): + * **`ValidateCheckCreate` now enforces `InvoiceID` as a Hash256**, not merely as a string. `sfInvoiceID` is a 256-bit hash and this same release added exactly that rule for `WalletLocator` in `AccountSet` and `SignerListSet`, so `CheckCreate` was the odd one out: a malformed value passed validation and only blew up later inside the codec, reporting an encoding error instead of a `ValidationException`. The exception message is unchanged (`CheckCreate: invalid InvoiceID`) + * the standalone stand's credential-protected ws port is bound to `127.0.0.1` instead of every interface — `rippled.cfg` carries those credentials in plain text, and the nightly stand already published it loopback-only + * `TestIConnectionStates` — both reconnect-exhaustion tests discarded the `Task.WhenAny` winner, so a run where the terminal event never arrived proceeded after the 30 s timeout and could still pass. They now assert the event task won, and that at least one reconnect was attempted + * `TestIProtocolFieldSets` sets `Expiration` on the mint-time NFT offer but never checked it read back; asserted now, closing the last unverified field of the corrected `NFTokenMint` set + * test-only tidying: the parse-floor literal is shared instead of duplicated (`RippledTransactionFormats.MinimumExpectedTransactions`), the common-field set both conformance surfaces subtract now comes from one helper (`RippledTransactionFormats.CommonFields`), and a redundant `Link` on the vendored fixture is dropped + ## 10.9.1.0 07/27/2026 * **Fix `account_tx` losing the payment amount and, on API v1, the whole transaction** — a silent regression introduced by the 10.3.0.0 `Newtonsoft.Json` → `System.Text.Json` migration; affects every release from 10.3.0.0 on: * `Payment`/`PaymentResponse.DeliverMax` — the private set-only alias that maps API v2's `DeliverMax` onto `Amount` was carried over from Newtonsoft (which deserializes attributed non-public members) but `System.Text.Json` skips non-public members without `[JsonInclude]`. Every Payment read through `AccountTransactions`, `TxV2` or the transaction streams came back with `Amount = null` — no exception, no diagnostic. `Tx()` was unaffected because it pins `ApiVersion = 1`, and `meta.delivered_amount` kept parsing correctly, which is why the loss went unnoticed. The alias stays set-only, so `DeliverMax` is still never serialized back out diff --git a/CLAUDE.md b/CLAUDE.md index 69b10aa9..3e4a6e4a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -204,11 +204,12 @@ Integration tests do **not** run on PRs into `dev` — `dev` uses a GitHub merge ### Release Process 1. Ensure all tests pass on `dev` -2. Update version in all `.csproj` files (`Xrpl`, `Xrpl.AddressCodec`, `Xrpl.BinaryCodec`, `Xrpl.Keypairs`) -3. Update `CHANGES.md` -4. Merge `dev` → `release` -5. NuGet publish triggers automatically on push to `release` -6. Create GitHub release with tag +2. Bump `` **only in the packages that actually changed** — not in all four. The base packages (`Xrpl.AddressCodec`, `Xrpl.BinaryCodec`, `Xrpl.Keypairs`) are consumed via `ProjectReference`, so a `Xrpl` package built at a newer version keeps depending on the already published base packages at their existing version. Leaving an untouched package behind is correct, not an oversight. Check with `git diff --stat origin/release...origin/dev -- Base/` before deciding +3. Choose the bump from the nature of the change: patch for a bugfix with no contract change, minor otherwise +4. Update `CHANGES.md` +5. Merge `dev` → `release` +6. NuGet publish triggers automatically on push to `release` +7. Create GitHub release with tag ### NuGet Packages Published diff --git a/Tests/Xrpl.Tests/Integration/TestIConnectionStates.cs b/Tests/Xrpl.Tests/Integration/TestIConnectionStates.cs index 121521e1..c7e0cdee 100644 --- a/Tests/Xrpl.Tests/Integration/TestIConnectionStates.cs +++ b/Tests/Xrpl.Tests/Integration/TestIConnectionStates.cs @@ -140,10 +140,12 @@ public async Task TestConnectionStateReconnect_InvalidServer() { } - await Task.WhenAny(tcs.Task, Task.Delay(TimeSpan.FromSeconds(30))); + Task terminal = await Task.WhenAny(tcs.Task, Task.Delay(TimeSpan.FromSeconds(30))); + Assert.AreSame(tcs.Task, terminal, "Reconnect exhaustion was not observed before the 30s timeout"); + await tcs.Task; Assert.IsTrue(stateChanges.Contains(XrpConnectionState.Connecting), "Should have Connecting state"); - Console.WriteLine($"Reconnect attempts: {reconnectAttempts}"); + Assert.IsTrue(reconnectAttempts >= 1, $"Expected at least one reconnect attempt, got {reconnectAttempts}"); await client.Disconnect(); } @@ -270,7 +272,10 @@ public async Task TestChangeServer_AfterMaxReconnectAttempts_NoNotConnectedExcep { } - await Task.WhenAny(disconnectedPermanently.Task, Task.Delay(TimeSpan.FromSeconds(30))); + Task terminal = await Task.WhenAny(disconnectedPermanently.Task, Task.Delay(TimeSpan.FromSeconds(30))); + Assert.AreSame(disconnectedPermanently.Task, terminal, + "Permanent disconnect was not observed before the 30s timeout"); + await disconnectedPermanently.Task; Assert.AreEqual(XrpConnectionState.Disconnected, client.connection.CurrentConnectionState, "Should be Disconnected after max reconnect attempts"); diff --git a/Tests/Xrpl.Tests/Integration/transactions/TestIProtocolFieldSets.cs b/Tests/Xrpl.Tests/Integration/transactions/TestIProtocolFieldSets.cs index 171bb682..dd63b0ad 100644 --- a/Tests/Xrpl.Tests/Integration/transactions/TestIProtocolFieldSets.cs +++ b/Tests/Xrpl.Tests/Integration/transactions/TestIProtocolFieldSets.cs @@ -166,6 +166,8 @@ public async Task TestINFTokenMint_OfferFieldsLandOnTheLedger() Assert.AreEqual("5000000", offer.Amount.Value, "the mint-time sell offer must carry Amount"); Assert.AreEqual(buyer.ClassicAddress, offer.Destination); + Assert.IsNotNull(offer.Expiration, "the mint-time sell offer must carry Expiration"); + Assert.AreEqual(expiration, offer.Expiration.Value); } finally { diff --git a/Tests/Xrpl.Tests/Models/RippledTransactionFormats.cs b/Tests/Xrpl.Tests/Models/RippledTransactionFormats.cs index 86a181e6..285460b6 100644 --- a/Tests/Xrpl.Tests/Models/RippledTransactionFormats.cs +++ b/Tests/Xrpl.Tests/Models/RippledTransactionFormats.cs @@ -1,8 +1,11 @@ using System; using System.Collections.Generic; using System.IO; +using System.Linq; using System.Text.RegularExpressions; +using Xrpl.BinaryCodec.Enums; + using TxFormat = Xrpl.Models.Transaction.TxFormat; namespace Xrpl.Tests.Models.Tests @@ -38,7 +41,19 @@ internal static class RippledTransactionFormats @"\{\s*sf(?\w+)\s*,\s*Soe(?\w+)", RegexOptions.Compiled); - private const int MinimumExpectedTransactions = 60; + /// + /// Lower bound on how many formats a healthy parse yields. Exposed so the guard test + /// asserts against the same number the parser enforces, instead of a second literal + /// that would drift when the fixture is re-pinned. + /// + internal const int MinimumExpectedTransactions = 60; + + /// + /// Fields shared by every transaction, declared once in the constructor. + /// rippled keeps them in a separate commonFields list, so both conformance surfaces + /// exclude them — they read the set from here so the two cannot drift apart. + /// + internal static HashSet CommonFields() => new TxFormat().Keys.ToHashSet(); internal static string FixturePath => Path.Combine(AppContext.BaseDirectory, "Fixtures", "transactions.macro"); diff --git a/Tests/Xrpl.Tests/Models/TestCheckCreate.cs b/Tests/Xrpl.Tests/Models/TestCheckCreate.cs index 60b1fd32..b895135a 100644 --- a/Tests/Xrpl.Tests/Models/TestCheckCreate.cs +++ b/Tests/Xrpl.Tests/Models/TestCheckCreate.cs @@ -141,6 +141,54 @@ public void TestUCheckCreate_InvoiceIDIsAHash256() Assert.AreEqual(invoiceId, decoded["InvoiceID"].GetValue()); } + + [TestMethod] + public async Task TestUCheckCreate_RejectsInvoiceIDThatIsNotA256BitHexValue() + { + // sfInvoiceID is Hash256. A string of the wrong length or with non-hex characters is + // malformed and must fail validation rather than blow up later inside the codec — + // the same rule SignerListSet and AccountSet already apply to WalletLocator. + string[] malformed = + { + "6F1DFD1D0FE8A32E40E1F2C05CF1C15545BAB56B617F9C6C2D63A6B704BEF59", // 63 chars + "6F1DFD1D0FE8A32E40E1F2C05CF1C15545BAB56B617F9C6C2D63A6B704BEF59BA", // 65 chars + "6F1DFD1D0FE8A32E40E1F2C05CF1C15545BAB56B617F9C6C2D63A6B704BEF5XZ", // non-hex + "", + }; + + foreach (string invoiceId in malformed) + { + Dictionary tx = new Dictionary + { + { "TransactionType", "CheckCreate" }, + { "Account", "rUn84CUYbNjRoTQ6mSW7BVJPSVJNLb1QLo" }, + { "Destination", "rfkE1aSy9G8Upk4JssnwBxhEv5p4mn2KTy" }, + { "SendMax", "100000000" }, + { "InvoiceID", invoiceId }, + { "Fee", "12" }, + }; + + await Helper.ThrowsExceptionAsync( + () => Validation.ValidateCheckCreate(tx), + "CheckCreate: invalid InvoiceID"); + } + } + + [TestMethod] + public async Task TestUCheckCreate_AcceptsA256BitHexInvoiceID() + { + Dictionary tx = new Dictionary + { + { "TransactionType", "CheckCreate" }, + { "Account", "rUn84CUYbNjRoTQ6mSW7BVJPSVJNLb1QLo" }, + { "Destination", "rfkE1aSy9G8Upk4JssnwBxhEv5p4mn2KTy" }, + { "SendMax", "100000000" }, + { "InvoiceID", "6f1dfd1d0fe8a32e40e1f2c05cf1c15545bab56b617f9c6c2d63a6b704bef59b" }, + { "Fee", "12" }, + }; + + await Validation.ValidateCheckCreate(tx); + } } } diff --git a/Tests/Xrpl.Tests/Models/TestUProtocolCompleteness.cs b/Tests/Xrpl.Tests/Models/TestUProtocolCompleteness.cs index a4962c39..367382b5 100644 --- a/Tests/Xrpl.Tests/Models/TestUProtocolCompleteness.cs +++ b/Tests/Xrpl.Tests/Models/TestUProtocolCompleteness.cs @@ -237,13 +237,15 @@ public async Task TestUMPTokenIssuanceCreate_MutableFlagsMask() /// /// The fields a format declares on top of the common set shared by every transaction. + /// The common set comes from , which + /// TestUTxFormatConformance also reads, so the two conformance surfaces stay in step. /// private static Dictionary TypeSpecificFields( BinaryCodec.Types.TransactionType transactionType) { - TxFormat common = new TxFormat(); + HashSet common = RippledTransactionFormats.CommonFields(); return TxFormat.Formats[transactionType] - .Where(entry => !common.ContainsKey(entry.Key)) + .Where(entry => !common.Contains(entry.Key)) .ToDictionary(entry => entry.Key, entry => entry.Value); } diff --git a/Tests/Xrpl.Tests/Models/TestUTxFormatConformance.cs b/Tests/Xrpl.Tests/Models/TestUTxFormatConformance.cs index ecd09f3e..4342a925 100644 --- a/Tests/Xrpl.Tests/Models/TestUTxFormatConformance.cs +++ b/Tests/Xrpl.Tests/Models/TestUTxFormatConformance.cs @@ -28,11 +28,14 @@ namespace Xrpl.Tests.Models.Tests public class TestUTxFormatConformance { /// - /// Fields shared by every transaction, declared once in the TxFormat constructor. - /// rippled keeps them in a separate commonFields list, so they are excluded on both sides. + /// Names of the fields shared by every transaction; excluded on both sides of the diff. + /// Derived from so this test and + /// TestUProtocolCompleteness cannot disagree about what "common" means. /// private static HashSet CommonFieldNames() => - new TxFormat().Keys.Select(field => field.Name).ToHashSet(StringComparer.Ordinal); + RippledTransactionFormats.CommonFields() + .Select(field => field.Name) + .ToHashSet(StringComparer.Ordinal); [TestMethod] public void TestUTxFormat_MatchesRippledTransactionsMacro() @@ -93,7 +96,10 @@ public void TestUTxFormatConformance_ParserFailsLoudlyOnAnEmptySource() // conformance test above pass against an empty table. Dictionary> parsed = RippledTransactionFormats.Parse(); - Assert.IsGreaterThan(60, parsed.Count, "the vendored macro must yield a full format table"); + Assert.IsGreaterThanOrEqualTo( + RippledTransactionFormats.MinimumExpectedTransactions, + parsed.Count, + "the vendored macro must yield a full format table"); Assert.IsTrue(parsed.ContainsKey("Payment"), "Payment must be present — the parser matched nothing sane"); Assert.IsTrue(parsed["Payment"].ContainsKey("Destination")); Assert.AreEqual(TxFormat.Requirement.Required, parsed["Payment"]["Destination"]); diff --git a/Tests/Xrpl.Tests/Xrpl.Tests.csproj b/Tests/Xrpl.Tests/Xrpl.Tests.csproj index e560d5a3..f076abe6 100644 --- a/Tests/Xrpl.Tests/Xrpl.Tests.csproj +++ b/Tests/Xrpl.Tests/Xrpl.Tests.csproj @@ -30,7 +30,7 @@ PreserveNewest - + PreserveNewest diff --git a/Xrpl/Client/connection.cs b/Xrpl/Client/connection.cs index 14b46e79..d443c3dd 100644 --- a/Xrpl/Client/connection.cs +++ b/Xrpl/Client/connection.cs @@ -2091,6 +2091,12 @@ private async Task ExecutePingCheckAsync(CancellationTokenSource cts) try { Debug.WriteLine($"{DateTime.Now}[PING-CHECK] Fire-and-forget keepalive ping (active connection)"); + + // Raw send: this bypasses RequestManager, so AdminUser/AdminPassword are NOT attached. + // Safe for ping specifically — rippled resolves the role per command, and a guest-level + // command is answered normally even on a port that sets admin_user/admin_password + // (only commands requiring Role::ADMIN get "forbidden / Bad credentials."). Anything + // needing admin must go through Request/GRequest instead of being added here. currentSocket?.SendMessage("{\"command\":\"ping\",\"id\":\"00000000-0000-0000-0000-000000000000\"}"); if (OnPing != null) { diff --git a/Xrpl/Models/Transactions/CheckCreate.cs b/Xrpl/Models/Transactions/CheckCreate.cs index 2351de68..5ac6064a 100644 --- a/Xrpl/Models/Transactions/CheckCreate.cs +++ b/Xrpl/Models/Transactions/CheckCreate.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Text.RegularExpressions; using System.Threading.Tasks; using System.Text.Json.Serialization; @@ -108,7 +109,11 @@ public static async Task ValidateCheckCreate(Dictionary tx) throw new ValidationException("CheckCreate: invalid DestinationTag"); if (tx.TryGetValue("Expiration", out var Expiration) && !Common.IsUInt32(Expiration)) throw new ValidationException("CheckCreate: invalid Expiration"); - if (tx.TryGetValue("InvoiceID", out var InvoiceID) && InvoiceID is not string { }) + // sfInvoiceID is a Hash256 — the same 256-bit rule SignerListSet and AccountSet apply to + // WalletLocator. Without the shape check a malformed string only fails later inside the + // codec, which reports an encoding error instead of a ValidationException. + if (tx.TryGetValue("InvoiceID", out var InvoiceID) && + (InvoiceID is not string invoiceId || !Regex.IsMatch(invoiceId, @"^[0-9A-Fa-f]{64}$"))) throw new ValidationException("CheckCreate: invalid InvoiceID");