diff --git a/Base/Xrpl.BinaryCodec/Types/StObject.cs b/Base/Xrpl.BinaryCodec/Types/StObject.cs index d48ca2d7..be67f1ba 100644 --- a/Base/Xrpl.BinaryCodec/Types/StObject.cs +++ b/Base/Xrpl.BinaryCodec/Types/StObject.cs @@ -190,7 +190,7 @@ public static StObject FromJsonStrict(JsonNode token) /// /// /// Strictness and the signing filter are separate concerns, and only look alike because - /// one flag used to carry both. still applies to the top + /// one flag used to carry both. FilterIsSigning still applies to the top /// level alone - dropping non-signing fields out of nested objects would change what gets /// signed, which is not what this fixes. /// diff --git a/CHANGES.md b/CHANGES.md index 93d0cfaf..3dd09a5d 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -19,6 +19,8 @@ This release makes the SDK stop misrepresenting what a node sent. It is a delibe | Untyped request | `Task> Request(...)` | `Task>> Request(...)` | | Signing helper | `GetSignedTx(tx, autofill, failHard, wallet, ...)` | `GetSignedTx(tx, autofill, wallet, ...)` — `failHard` never did anything here; pass it to `Submit`/`SubmitAndWait`, which do submit | | Unknown member in a nested object | dropped from the blob in silence | `InvalidJsonException` on the signing path, at any depth | +| Path step type | `Xrpl.Models.Methods.Path` | `Xrpl.Models.Common.PathStep` — `List>` becomes `List>` | +| Model helpers | `Xrpl.Models.Utils.Index` | `Xrpl.Models.Utils.ModelUtils` | | Stream events | `client.connection.OnTransaction += …` | also `client.OnTransaction += …` — on `IXrplClient` now; the old form still works | | `IXrplClient.connection` | `{ get; set; }` | `{ get; }` — assigning it would strand handlers on the old object | | Dropped stream events | invisible | `client.DroppedStreamMessages` counts them; `StreamMessageQueueCapacity` sizes the queue | @@ -44,6 +46,17 @@ The raw-response work, all five levels landing together. The problem: a consumer The entries below are grouped by what changed, not by the order the levels were built in. The short version of what will not compile is in the table above. +* **`Models.Path` is a path step, and now says so: `Xrpl.Models.Common.PathStep`** (#117). The type describes one step of one path, not a path, and the old name cost three separate things. + * **it collided with `System.IO.Path`.** The proof was already in this repository: `TestUResponseFidelity.cs` was the one test file importing both `System.IO` and `Xrpl.Models.Methods`, and it had to write `System.IO.Path.Combine` in three places while its neighbours wrote `Path.Combine`. Those three qualifications are gone in this change, which is the check that the collision is gone with them. Consumers paid more: with `ImplicitUsings` on, a single `using Xrpl.Models.Methods;` was enough to turn any `Path.Combine` in the file into `CS0104` + * **it collided with `Xrpl.BinaryCodec.Types.Path`**, which is a whole path — so one name meant a container in one half of the SDK and its element in the other. No file imported both, so nothing had failed yet; the codec keeps its `Path`, where the name is right + * **everything around it already said step**: `PathStepType`, `Validation.IsPathStep`, `TestUPathStep`, and xrpl.js, where this is `PathStep` and `Path = PathStep[]`. `List>` read as a list of lists of paths while meaning a list of paths + * the wire format does not change. Only the C# name moves; the `[JsonPropertyName]` on `account`, `currency`, `issuer`, `mpt_issuance_id` and `type` are untouched, so serialization and signing are identical + * **no `[Obsolete]` bridge, because there cannot be one**: `[Obsolete] class Path : PathStep {}` would not help, since generics are invariant and `List>` still would not convert. It would add a type to the public surface and compile nothing that did not compile anyway + * two files stopped needing `using Xrpl.Models.Methods;` altogether once the type moved - `Payment.cs` and `TestUPathStep.cs` - so they no longer drag in the namespace that caused the collision in the first place. Both `using` directives are deleted rather than left as decoration + * **migration**: replace `Path` with `PathStep` and add `using Xrpl.Models.Common;`, or put `using Path = Xrpl.Models.Common.PathStep;` at the top of the file for now +* **`Xrpl.Models.Utils.Index` is now `ModelUtils`** (#117, the same defect one layer over). `Index` was a calque of the barrel file `utils/index.ts` it was ported from, and it collides with `System.Index`, which is in scope in every file whether anyone asked for it or not. `Payment.cs` carried `using Index = Xrpl.Models.Utils.Index;` — an alias that existed for no other reason than to work around that collision, and is now deleted. The class also finally matches `ModelUtils.cs`, the file it always lived in +* **`Xrpl` goes to 11.0.0.0** (from 10.12.0.0), the major this release has been heading for since the first breaking change in it. `Xrpl.BinaryCodec` is already at 11.0.0.0; `Xrpl.AddressCodec` and `Xrpl.Keypairs` stay at 10.9.0.0, untouched since the last release + * **Fourteen response fields the node sends now have typed properties** (#106). Thirteen was the count in the report; measuring found one more, and the arithmetic below is ten plus one plus two plus one. Unknown-field capture made the loss visible instead of silent; these were the ones it found. Capture is the safety net, declaring them is the fix, and a field counts as done only when it is a declared property **and** gone from `UnknownFields` - either half alone can pass while the other fails. * `ServerInfo.Info` gains **ten**, not the seven the report listed. Measuring against a node rather than working from the list found three more - `git`, `node_size` and `validator_list`. The test asserts the whole capture is empty rather than a list of names, precisely so it cannot miss what nobody thought of * the types came from a node too, not from documentation. `server_state_duration_us` is a **string** in `server_info` while the same field is a number in `server_state`; `initial_sync_duration_us`, `jq_trans_overflow`, `peer_disconnects`, `peer_disconnects_resources` and `time` are all strings. `ports` is a list of `{port, protocol[]}`, and `git` and `validator_list` are objects, so three small types come with them diff --git a/Tests/Xrpl.Tests/BinaryCodec/TestUStrictNestedFields.cs b/Tests/Xrpl.Tests/BinaryCodec/TestUStrictNestedFields.cs index 930411a2..144644fd 100644 --- a/Tests/Xrpl.Tests/BinaryCodec/TestUStrictNestedFields.cs +++ b/Tests/Xrpl.Tests/BinaryCodec/TestUStrictNestedFields.cs @@ -241,7 +241,7 @@ public void TestUUnknownMemberInAPathStepIsRefused() /// /// /// The trap in refusing unknown members here. ripple_path_find answers with a - /// type on every step, this SDK declares it on Path and emits it back out of + /// type on every step, this SDK declares it on PathStep and emits it back out of /// PathHop.ToJson, so a path taken from a response and put into a payment carries it. /// The byte is synthesised from which of account, currency and issuer are present, so the /// member is redundant rather than unknown - refusing it would break the ordinary diff --git a/Tests/Xrpl.Tests/Integration/requests/TestIPathPayment.cs b/Tests/Xrpl.Tests/Integration/requests/TestIPathPayment.cs index 900b0ca8..cebced82 100644 --- a/Tests/Xrpl.Tests/Integration/requests/TestIPathPayment.cs +++ b/Tests/Xrpl.Tests/Integration/requests/TestIPathPayment.cs @@ -305,8 +305,8 @@ await IntegrationTestConfig.TryFundWalletsAsync(client, nodeType, { for (int i = 0; i < alt.PathsComputed.Count; i++) { - List steps = alt.PathsComputed[i]; - foreach (Path step in steps) + List steps = alt.PathsComputed[i]; + foreach (PathStep step in steps) { Console.WriteLine($"[CrossCurrency] step: type={step.Type} account={step.Account} currency={step.CurrencyCode} issuer={step.Issuer}"); } diff --git a/Tests/Xrpl.Tests/Models/TestModelUtils.cs b/Tests/Xrpl.Tests/Models/TestModelUtils.cs index 0aeebd61..bf4c2dd5 100644 --- a/Tests/Xrpl.Tests/Models/TestModelUtils.cs +++ b/Tests/Xrpl.Tests/Models/TestModelUtils.cs @@ -33,11 +33,11 @@ public async Task TestVerifyValid_isFlagEnabled() //verifies a flag is enabled flags |= flag1 | flag2; - Assert.IsTrue(Index.IsFlagEnabled(flags, flag1)); + Assert.IsTrue(ModelUtils.IsFlagEnabled(flags, flag1)); //verifies a flag is not enabled flags = 0x00000000; flags |= flag2; - Assert.IsFalse(Index.IsFlagEnabled(flags, flag1)); + Assert.IsFalse(ModelUtils.IsFlagEnabled(flags, flag1)); } [TestMethod] public async Task TestVerifyValid_setTransactionFlagsToNumber() diff --git a/Tests/Xrpl.Tests/Models/TestUOutgoingShapesCarryNoCapture.cs b/Tests/Xrpl.Tests/Models/TestUOutgoingShapesCarryNoCapture.cs index 8278dcf2..caf77db9 100644 --- a/Tests/Xrpl.Tests/Models/TestUOutgoingShapesCarryNoCapture.cs +++ b/Tests/Xrpl.Tests/Models/TestUOutgoingShapesCarryNoCapture.cs @@ -22,7 +22,7 @@ namespace XrplTests.Xrpl.Models /// only to the top level, so a nested unknown member reaches the displayed tx_json but /// not the signed blob. Show one, sign another. /// - /// This is not hypothetical: Methods.Path (reaching Payment.Paths), + /// This is not hypothetical: Common.PathStep (reaching Payment.Paths), /// AuthAccount (AMMBid) and the AuthorizeCredential* pair /// (DepositPreauth) all carried capture until a review caught it. They were missed /// because the exclusion list was written by type name, while the property that matters is @@ -43,7 +43,7 @@ public class TestUOutgoingShapesCarryNoCapture /// Deliberately not . Capture arrives by inheritance /// far more often than by declaration: 47 method models get it from /// alone, and the defect that prompted this test was - /// exactly that - Methods.Path deriving from that base. A DeclaredOnly version of + /// exactly that - the path step deriving from that base. A DeclaredOnly version of /// this check stayed green with the defect reintroduced. /// private static bool CarriesCapture(Type type) => @@ -54,10 +54,10 @@ private static bool CarriesCapture(Type type) => /// Every model type a property type can hold, unwrapping arrays and generics to any depth. /// /// - /// Recursive on purpose. Payment.Paths is List<List<Path>>: peeling - /// one level yields List<Path>, which lives outside Xrpl.Models and gets - /// discarded, so Path is never reached. A single-level version of this walk passed - /// while Path carried capture - the very defect that prompted this test. + /// Recursive on purpose. Payment.Paths is List<List<PathStep>>: peeling + /// one level yields List<PathStep>, which lives outside Xrpl.Models and gets + /// discarded, so PathStep is never reached. A single-level version of this walk passed + /// while PathStep carried capture - the very defect that prompted this test. /// private static IEnumerable Unwrap(Type type) { diff --git a/Tests/Xrpl.Tests/Models/TestUPathStep.cs b/Tests/Xrpl.Tests/Models/TestUPathStep.cs index 8e565c06..4f2ef96a 100644 --- a/Tests/Xrpl.Tests/Models/TestUPathStep.cs +++ b/Tests/Xrpl.Tests/Models/TestUPathStep.cs @@ -4,8 +4,8 @@ using System.Text.Json; using Xrpl.Client.Json; +using Xrpl.Models.Common; using Xrpl.Models.Enums; -using Xrpl.Models.Methods; using Xrpl.Models.Transactions; namespace XrplTests.Xrpl.Models @@ -28,7 +28,7 @@ public void TestUPathStepTypeDeserializesAsFlags() // shape of mainnet tx 1D813B78FC55ABF9054AEBD2AF9DD7C90361F9985B7897E8E9A592D63BF0CC43 string json = @"{""currency"":""4249547800000000000000000000000000000000"",""issuer"":""rBitcoiNXev8VoVxV7pwoQx1sSfonVP9i3"",""type"":48}"; - Path step = JsonSerializer.Deserialize(json, XrplJsonOptions.Default); + PathStep step = JsonSerializer.Deserialize(json, XrplJsonOptions.Default); Assert.AreEqual(PathStepType.Currency | PathStepType.Issuer, step.Type); Assert.IsTrue(step.Type.Value.HasFlag(PathStepType.Issuer)); @@ -41,7 +41,7 @@ public void TestUPathStepMptTypeDeserializesAsFlags() { string json = @"{""mpt_issuance_id"":""" + MptIssuanceId + @""",""issuer"":""rBitcoiNXev8VoVxV7pwoQx1sSfonVP9i3"",""type"":96}"; - Path step = JsonSerializer.Deserialize(json, XrplJsonOptions.Default); + PathStep step = JsonSerializer.Deserialize(json, XrplJsonOptions.Default); Assert.AreEqual(MptIssuanceId, step.MPTokenIssuanceID); Assert.AreEqual(PathStepType.MPTokenIssuanceID | PathStepType.Issuer, step.Type); @@ -51,7 +51,7 @@ public void TestUPathStepMptTypeDeserializesAsFlags() [TestCategory("TestU")] public void TestUPathStepTypeStaysNumericOnTheWire() { - Path step = new Path + PathStep step = new PathStep { CurrencyCode = "4249547800000000000000000000000000000000", Issuer = "rBitcoiNXev8VoVxV7pwoQx1sSfonVP9i3", @@ -68,7 +68,7 @@ public void TestUPathStepTypeStaysNumericOnTheWire() public void TestUPathStepUndeclaredTypeBitSurvives() { // a future protocol bit the enum does not name must not break deserialization - Path step = JsonSerializer.Deserialize(@"{""type"":176}", XrplJsonOptions.Default); + PathStep step = JsonSerializer.Deserialize(@"{""type"":176}", XrplJsonOptions.Default); Assert.AreEqual(176u, (uint)step.Type.Value); Assert.IsTrue(step.Type.Value.HasFlag(PathStepType.Issuer)); @@ -82,7 +82,7 @@ public void TestUPathStepIgnoresLegacyTypeHex() // model; a response from an ancient server must still deserialize, with the key ignored string json = @"{""currency"":""USD"",""issuer"":""rBitcoiNXev8VoVxV7pwoQx1sSfonVP9i3"",""type"":48,""type_hex"":""0000000000000030""}"; - Path step = JsonSerializer.Deserialize(json, XrplJsonOptions.Default); + PathStep step = JsonSerializer.Deserialize(json, XrplJsonOptions.Default); Assert.AreEqual(PathStepType.Currency | PathStepType.Issuer, step.Type); Assert.AreEqual("USD", step.CurrencyCode); @@ -120,7 +120,7 @@ private static Dictionary Step(params (string Key, object Value) [TestCategory("TestU")] public void TestUPathStepWithoutTypeIsNull() { - Path step = JsonSerializer.Deserialize(@"{""account"":""rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn""}", XrplJsonOptions.Default); + PathStep step = JsonSerializer.Deserialize(@"{""account"":""rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn""}", XrplJsonOptions.Default); Assert.IsNull(step.Type); } diff --git a/Tests/Xrpl.Tests/Models/TestUResponseFidelity.cs b/Tests/Xrpl.Tests/Models/TestUResponseFidelity.cs index b05a314f..7412b10e 100644 --- a/Tests/Xrpl.Tests/Models/TestUResponseFidelity.cs +++ b/Tests/Xrpl.Tests/Models/TestUResponseFidelity.cs @@ -52,7 +52,7 @@ namespace Xrpl.Tests.Models.Tests public class TestUResponseFidelity { private static readonly string ResponsesDirectory = - System.IO.Path.Combine(AppContext.BaseDirectory, "Fixtures", "Responses"); + Path.Combine(AppContext.BaseDirectory, "Fixtures", "Responses"); /// /// Corpus file -> the model type XrplClient actually deserializes that command's @@ -122,7 +122,7 @@ public void TestUEveryCorpusFileHasAModelMapping() Assert.IsTrue(corpusFiles.Length > 0, $"no .json fixtures found under {ResponsesDirectory}"); List unmapped = corpusFiles - .Select(System.IO.Path.GetFileName) + .Select(Path.GetFileName) .Where(name => !Models.ContainsKey(name)) .OrderBy(name => name, StringComparer.Ordinal) .ToList(); @@ -149,7 +149,7 @@ public void TestUCorpusRoundTripIsFaithful() { string file = entry.Key; Type modelType = entry.Value; - string fixturePath = System.IO.Path.Combine(ResponsesDirectory, file); + string fixturePath = Path.Combine(ResponsesDirectory, file); Assert.IsTrue(File.Exists(fixturePath), $"{file}: mapped in Models but the fixture file is missing at {fixturePath}"); diff --git a/Xrpl/Models/Methods/Path.cs b/Xrpl/Models/Common/PathStep.cs similarity index 77% rename from Xrpl/Models/Methods/Path.cs rename to Xrpl/Models/Common/PathStep.cs index 8c2c5f29..52ac4473 100644 --- a/Xrpl/Models/Methods/Path.cs +++ b/Xrpl/Models/Common/PathStep.cs @@ -2,13 +2,24 @@ using Xrpl.Models.Enums; //https://github.com/XRPLF/xrpl.js/blob/b20c05c3680d80344006d20c44b4ae1c3b0ffcac/packages/xrpl/src/models/common/index.ts#L62 //https://xrpl.org/paths.html#path-steps -namespace Xrpl.Models.Methods +namespace Xrpl.Models.Common { /// - /// A path set is an array.
- /// Each member of the path set is another array that represents an individual path.
- /// Each member of a path is an object that specifies the step. + /// One step of one path: where the payment goes next, not the whole route. ///
+ /// + /// A path set is an array of paths, and a path is an array of these. The nesting reads + /// correctly now that the name does: List<List<PathStep>> is a list of paths, + /// where the old List<List<Path>> read as a list of lists of paths. + /// + /// Named Path until 11.0.0.0, which collided with - any file + /// with using Xrpl.Models.Methods; and implicit usings on could not write + /// Path.Combine - and with Xrpl.BinaryCodec.Types.Path, which is a whole path + /// rather than a step, so the same name meant a container in one half of the SDK and its + /// element in the other. Everything around it already said step: , + /// Validation.IsPathStep, and xrpl.js, where this is PathStep. + /// + /// // No unknown-field capture here on purpose: a Path step is not only read off a // ripple_path_find/path_find response, it is fed straight back into an outgoing // Payment (Transactions/Payment.cs Paths) and PathFindCreateRequest. Capturing @@ -17,7 +28,7 @@ namespace Xrpl.Models.Methods // passes signingOnly only to the top level, so a nested unknown member reaches the // displayed tx_json but not the signed blob. Show-one-sign-another, the exact // failure this branch exists to remove, arriving from the outgoing side. - public class Path//todo rename to path steps? + public class PathStep { /// /// (Optional) If present, this path step represents rippling through the specified address.
diff --git a/Xrpl/Models/Methods/PathFind.cs b/Xrpl/Models/Methods/PathFind.cs index 72009b34..06159b31 100644 --- a/Xrpl/Models/Methods/PathFind.cs +++ b/Xrpl/Models/Methods/PathFind.cs @@ -18,14 +18,14 @@ public class PathAlternative : BaseMethodResult /// Array of arrays of objects defining payment paths. ///
[JsonPropertyName("paths_computed")] - public List> PathsComputed { get; set; } + public List> PathsComputed { get; set; } /// /// (Deprecated) Array of arrays of objects defining canonical payment paths.
/// May be present in server responses but should be disregarded. ///
[JsonPropertyName("paths_canonical")] - public List> PathsCanonical { get; set; } + public List> PathsCanonical { get; set; } /// /// Currency Amount that the source would have to send along this path @@ -161,7 +161,7 @@ public PathFindCreateRequest(string sourceAccount, string destinationAccount, Cu /// or to check the overall cost to make a payment along a certain path. /// [JsonPropertyName("paths")] - public List> Paths { get; set; } + public List> Paths { get; set; } } /// diff --git a/Xrpl/Models/Transactions/NFTokenCreateOffer.cs b/Xrpl/Models/Transactions/NFTokenCreateOffer.cs index 31bebef6..e80d744b 100644 --- a/Xrpl/Models/Transactions/NFTokenCreateOffer.cs +++ b/Xrpl/Models/Transactions/NFTokenCreateOffer.cs @@ -171,7 +171,7 @@ public static Task ValidateNFTokenCreateOffer(Dictionary tx) if (tx.TryGetValue("Flags", out var Flags) && Flags is uint {} flags - && Utils.Index.IsFlagEnabled(flags,(uint)NFTokenCreateOfferFlags.tfSellNFToken)) + && Utils.ModelUtils.IsFlagEnabled(flags,(uint)NFTokenCreateOfferFlags.tfSellNFToken)) { ValidateNFTokenSellOfferCases(tx); } diff --git a/Xrpl/Models/Transactions/Payment.cs b/Xrpl/Models/Transactions/Payment.cs index 3ac165e4..b28aa3aa 100644 --- a/Xrpl/Models/Transactions/Payment.cs +++ b/Xrpl/Models/Transactions/Payment.cs @@ -8,10 +8,8 @@ using Xrpl.Client.Json.Converters; using Xrpl.Models.Common; using Xrpl.Models.Enums; -using Xrpl.Models.Methods; using Xrpl.Models.Utils; -using Index = Xrpl.Models.Utils.Index; // https://github.com/XRPLF/xrpl.js/blob/main/packages/xrpl/src/models/transactions/payment.ts @@ -124,7 +122,7 @@ private Currency? DeliverMax public string InvoiceID { get; set; } /// - public List> Paths { get; set; } + public List> Paths { get; set; } /// [JsonConverter(typeof(CurrencyConverter))] @@ -204,7 +202,7 @@ public interface IPayment : ITransactionCommon, IDestination /// Array of payment paths to be used for this transaction.
/// Must be omitted for XRP-to-XRP transactions. ///
- List> Paths { get; set; } + List> Paths { get; set; } /// /// Highest amount of source currency this transaction is allowed to cost, including transfer fees, exchange rates, and slippage.
/// Does not include the XRP destroyed as a cost for submitting the transaction.
@@ -337,7 +335,7 @@ private Currency DeliverMax public string InvoiceID { get; set; } /// - public List> Paths { get; set; } + public List> Paths { get; set; } /// [JsonConverter(typeof(CurrencyConverter))] @@ -417,7 +415,7 @@ public static Task CheckPartialPayment(Dictionary tx) } bool isTfPartialPayment = flags is uint uFlag - ? Index.IsFlagEnabled(uFlag, (uint)PaymentFlags.tfPartialPayment) + ? ModelUtils.IsFlagEnabled(uFlag, (uint)PaymentFlags.tfPartialPayment) : flags is PaymentFlags pf ? pf == PaymentFlags.tfPartialPayment : flags is Dictionary flagDict && CheckFlag(flagDict, "tfPartialPayment"); diff --git a/Xrpl/Models/Utils/ModelUtils.cs b/Xrpl/Models/Utils/ModelUtils.cs index c8caabdc..2a783046 100644 --- a/Xrpl/Models/Utils/ModelUtils.cs +++ b/Xrpl/Models/Utils/ModelUtils.cs @@ -6,9 +6,18 @@ using System.Collections.Generic; using System.Linq; -namespace Xrpl.Models.Utils //todo ? +namespace Xrpl.Models.Utils { - public static class Index + /// + /// Helpers shared by the models. + /// + /// + /// Called Index until 11.0.0.0 - a calque of the barrel file utils/index.ts it was + /// ported from, and a name that collides with , which is in scope in + /// every file whether anyone asked for it or not. The class now matches the file it has always + /// lived in. + /// + public static class ModelUtils { /// /// Verify that all fields of an object are in fields. diff --git a/Xrpl/Xrpl.csproj b/Xrpl/Xrpl.csproj index 1c78a156..cf08d66b 100644 --- a/Xrpl/Xrpl.csproj +++ b/Xrpl/Xrpl.csproj @@ -14,7 +14,7 @@ Apache-2.0 https://github.com/StaticBit-io/XrplCSharp XrplCSharp - 10.12.0.0 + 11.0.0.0