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
2 changes: 1 addition & 1 deletion Base/Xrpl.BinaryCodec/Types/StObject.cs
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ public static StObject FromJsonStrict(JsonNode token)
/// </para>
/// <para>
/// Strictness and the signing filter are separate concerns, and only look alike because
/// one flag used to carry both. <see cref="FilterIsSigning"/> still applies to the top
/// one flag used to carry both. <c>FilterIsSigning</c> 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.
/// </para>
Expand Down
13 changes: 13 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ This release makes the SDK stop misrepresenting what a node sent. It is a delibe
| Untyped request | `Task<Dictionary<string, object>> Request(...)` | `Task<XrplResponse<Dictionary<string, object>>> 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<List<Path>>` becomes `List<List<PathStep>>` |
| 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 |
Expand All @@ -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<List<Path>>` 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<List<Path>>` 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
Expand Down
2 changes: 1 addition & 1 deletion Tests/Xrpl.Tests/BinaryCodec/TestUStrictNestedFields.cs
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ public void TestUUnknownMemberInAPathStepIsRefused()
/// </summary>
/// <remarks>
/// The trap in refusing unknown members here. <c>ripple_path_find</c> answers with a
/// <c>type</c> on every step, this SDK declares it on <c>Path</c> and emits it back out of
/// <c>type</c> on every step, this SDK declares it on <c>PathStep</c> and emits it back out of
/// <c>PathHop.ToJson</c>, 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
Expand Down
4 changes: 2 additions & 2 deletions Tests/Xrpl.Tests/Integration/requests/TestIPathPayment.cs
Original file line number Diff line number Diff line change
Expand Up @@ -305,8 +305,8 @@ await IntegrationTestConfig.TryFundWalletsAsync(client, nodeType,
{
for (int i = 0; i < alt.PathsComputed.Count; i++)
{
List<Path> steps = alt.PathsComputed[i];
foreach (Path step in steps)
List<PathStep> 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}");
}
Expand Down
4 changes: 2 additions & 2 deletions Tests/Xrpl.Tests/Models/TestModelUtils.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
12 changes: 6 additions & 6 deletions Tests/Xrpl.Tests/Models/TestUOutgoingShapesCarryNoCapture.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ namespace XrplTests.Xrpl.Models
/// only to the top level, so a nested unknown member reaches the displayed <c>tx_json</c> but
/// not the signed blob. Show one, sign another.
/// <para>
/// This is not hypothetical: <c>Methods.Path</c> (reaching <c>Payment.Paths</c>),
/// This is not hypothetical: <c>Common.PathStep</c> (reaching <c>Payment.Paths</c>),
/// <c>AuthAccount</c> (<c>AMMBid</c>) and the <c>AuthorizeCredential*</c> pair
/// (<c>DepositPreauth</c>) 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
Expand All @@ -43,7 +43,7 @@ public class TestUOutgoingShapesCarryNoCapture
/// Deliberately not <see cref="BindingFlags.DeclaredOnly"/>. Capture arrives by inheritance
/// far more often than by declaration: 47 method models get it from
/// <see cref="Methods.BaseMethodResult"/> alone, and the defect that prompted this test was
/// exactly that - <c>Methods.Path</c> 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.
/// </remarks>
private static bool CarriesCapture(Type type) =>
Expand All @@ -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.
/// </summary>
/// <remarks>
/// Recursive on purpose. <c>Payment.Paths</c> is <c>List&lt;List&lt;Path&gt;&gt;</c>: peeling
/// one level yields <c>List&lt;Path&gt;</c>, which lives outside Xrpl.Models and gets
/// discarded, so <c>Path</c> is never reached. A single-level version of this walk passed
/// while <c>Path</c> carried capture - the very defect that prompted this test.
/// Recursive on purpose. <c>Payment.Paths</c> is <c>List&lt;List&lt;PathStep&gt;&gt;</c>: peeling
/// one level yields <c>List&lt;PathStep&gt;</c>, which lives outside Xrpl.Models and gets
/// discarded, so <c>PathStep</c> is never reached. A single-level version of this walk passed
/// while <c>PathStep</c> carried capture - the very defect that prompted this test.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
/// </remarks>
private static IEnumerable<Type> Unwrap(Type type)
{
Expand Down
14 changes: 7 additions & 7 deletions Tests/Xrpl.Tests/Models/TestUPathStep.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -28,7 +28,7 @@ public void TestUPathStepTypeDeserializesAsFlags()
// shape of mainnet tx 1D813B78FC55ABF9054AEBD2AF9DD7C90361F9985B7897E8E9A592D63BF0CC43
string json = @"{""currency"":""4249547800000000000000000000000000000000"",""issuer"":""rBitcoiNXev8VoVxV7pwoQx1sSfonVP9i3"",""type"":48}";

Path step = JsonSerializer.Deserialize<Path>(json, XrplJsonOptions.Default);
PathStep step = JsonSerializer.Deserialize<PathStep>(json, XrplJsonOptions.Default);

Assert.AreEqual(PathStepType.Currency | PathStepType.Issuer, step.Type);
Assert.IsTrue(step.Type.Value.HasFlag(PathStepType.Issuer));
Expand All @@ -41,7 +41,7 @@ public void TestUPathStepMptTypeDeserializesAsFlags()
{
string json = @"{""mpt_issuance_id"":""" + MptIssuanceId + @""",""issuer"":""rBitcoiNXev8VoVxV7pwoQx1sSfonVP9i3"",""type"":96}";

Path step = JsonSerializer.Deserialize<Path>(json, XrplJsonOptions.Default);
PathStep step = JsonSerializer.Deserialize<PathStep>(json, XrplJsonOptions.Default);

Assert.AreEqual(MptIssuanceId, step.MPTokenIssuanceID);
Assert.AreEqual(PathStepType.MPTokenIssuanceID | PathStepType.Issuer, step.Type);
Expand All @@ -51,7 +51,7 @@ public void TestUPathStepMptTypeDeserializesAsFlags()
[TestCategory("TestU")]
public void TestUPathStepTypeStaysNumericOnTheWire()
{
Path step = new Path
PathStep step = new PathStep
{
CurrencyCode = "4249547800000000000000000000000000000000",
Issuer = "rBitcoiNXev8VoVxV7pwoQx1sSfonVP9i3",
Expand All @@ -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<Path>(@"{""type"":176}", XrplJsonOptions.Default);
PathStep step = JsonSerializer.Deserialize<PathStep>(@"{""type"":176}", XrplJsonOptions.Default);

Assert.AreEqual(176u, (uint)step.Type.Value);
Assert.IsTrue(step.Type.Value.HasFlag(PathStepType.Issuer));
Expand All @@ -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<Path>(json, XrplJsonOptions.Default);
PathStep step = JsonSerializer.Deserialize<PathStep>(json, XrplJsonOptions.Default);

Assert.AreEqual(PathStepType.Currency | PathStepType.Issuer, step.Type);
Assert.AreEqual("USD", step.CurrencyCode);
Expand Down Expand Up @@ -120,7 +120,7 @@ private static Dictionary<string, object> Step(params (string Key, object Value)
[TestCategory("TestU")]
public void TestUPathStepWithoutTypeIsNull()
{
Path step = JsonSerializer.Deserialize<Path>(@"{""account"":""rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn""}", XrplJsonOptions.Default);
PathStep step = JsonSerializer.Deserialize<PathStep>(@"{""account"":""rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn""}", XrplJsonOptions.Default);

Assert.IsNull(step.Type);
}
Expand Down
6 changes: 3 additions & 3 deletions Tests/Xrpl.Tests/Models/TestUResponseFidelity.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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");

/// <summary>
/// Corpus file -> the model type XrplClient actually deserializes that command's
Expand Down Expand Up @@ -122,7 +122,7 @@ public void TestUEveryCorpusFileHasAModelMapping()
Assert.IsTrue(corpusFiles.Length > 0, $"no .json fixtures found under {ResponsesDirectory}");

List<string> unmapped = corpusFiles
.Select(System.IO.Path.GetFileName)
.Select(Path.GetFileName)
.Where(name => !Models.ContainsKey(name))
.OrderBy(name => name, StringComparer.Ordinal)
.ToList();
Expand All @@ -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}");

Expand Down
21 changes: 16 additions & 5 deletions Xrpl/Models/Methods/Path.cs → Xrpl/Models/Common/PathStep.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
/// <summary>
/// A path set is an array.<br/>
/// Each member of the path set is another array that represents an individual path.<br/>
/// 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.
/// </summary>
/// <remarks>
/// 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: <c>List&lt;List&lt;PathStep&gt;&gt;</c> is a list of paths,
/// where the old <c>List&lt;List&lt;Path&gt;&gt;</c> read as a list of lists of paths.
/// <para>
/// Named <c>Path</c> until 11.0.0.0, which collided with <see cref="System.IO.Path"/> - any file
/// with <c>using Xrpl.Models.Methods;</c> and implicit usings on could not write
/// <c>Path.Combine</c> - and with <c>Xrpl.BinaryCodec.Types.Path</c>, 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: <see cref="PathStepType"/>,
/// <c>Validation.IsPathStep</c>, and xrpl.js, where this is <c>PathStep</c>.
/// </para>
/// </remarks>
// 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
Expand All @@ -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
{
/// <summary>
/// (Optional) If present, this path step represents rippling through the specified address.<br/>
Expand Down
6 changes: 3 additions & 3 deletions Xrpl/Models/Methods/PathFind.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,14 @@ public class PathAlternative : BaseMethodResult
/// Array of arrays of objects defining payment paths.
/// </summary>
[JsonPropertyName("paths_computed")]
public List<List<Path>> PathsComputed { get; set; }
public List<List<PathStep>> PathsComputed { get; set; }

/// <summary>
/// (Deprecated) Array of arrays of objects defining canonical payment paths.<br/>
/// May be present in server responses but should be disregarded.
/// </summary>
[JsonPropertyName("paths_canonical")]
public List<List<Path>> PathsCanonical { get; set; }
public List<List<PathStep>> PathsCanonical { get; set; }

/// <summary>
/// Currency Amount that the source would have to send along this path
Expand Down Expand Up @@ -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.
/// </summary>
[JsonPropertyName("paths")]
public List<List<Path>> Paths { get; set; }
public List<List<PathStep>> Paths { get; set; }
}

/// <summary>
Expand Down
2 changes: 1 addition & 1 deletion Xrpl/Models/Transactions/NFTokenCreateOffer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ public static Task ValidateNFTokenCreateOffer(Dictionary<string, object> 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);
}
Expand Down
Loading
Loading