diff --git a/CHANGELOG.md b/CHANGELOG.md index da05a75..a680410 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,6 +38,12 @@ is ported to C#/.NET 10, test-first and verified to the digit against the upstre files, contributor & porting guides, and tag-driven NuGet packaging (SourceLink + symbols + MinVer). ### Changed +- **Nullable-reference hardening of the public API (pre-1.0.0).** Option/params types now carry explicit null + contracts taken from the upstream TypeScript interfaces: upstream-required fields are `required` in C#, + upstream-optional fields (`x?: T`) are nullable. `CurrencyAmount.Wrapped()` and `.AsBaseCurrency()` are now + non-nullable (they provably never return null), strengthening the contract for callers. The solution builds + with **zero compiler warnings**. Consumers using object initializers may need to supply fields now marked + `required` — a deliberate source-level tightening done before the stable release. - Test assertions migrated from FluentAssertions to **AwesomeAssertions** (Apache-2.0 community fork) to avoid FluentAssertions v8's commercial license. Test-only; no effect on the shipped package. - Runtime dependencies updated to `Nethereum` 6.1.0 and `ExtendedNumerics.BigRational` 3000.0.2.132, @@ -46,6 +52,15 @@ is ported to C#/.NET 10, test-first and verified to the digit against the upstre to provide Ed25519 for the tamperproof-transactions port. ### Fixed +- **V3 `SwapRouter` input-token-permit path was unusable.** `SwapOptions.InputTokenPermit` was typed against an + empty stub class, so the only assignable value failed `SelfPermit.EncodePermit`'s type tests and always threw + `"Invalid permit options"` — while the valid permit types could not be assigned to it at all. The upstream + `PermitOptions` union is now modelled as `SelfPermit.IPermitOptions`, implemented by both + `StandardPermitArguments` and `AllowedPermitArguments`. Every permit option (`InputTokenPermit`, + `OutputTokenPermit`, `Token0Permit`, `Token1Permit`) is typed against it instead of `object`, so misuse is now + a compile error. Pinned by a new regression test; upstream ships no test for this path. +- `PoolTests.BigNums_CorrectlyHandlesTwoBigIntegers` awaited its `GetInputAmount` call — previously the + unawaited `Task` swallowed exceptions, so the test could pass vacuously. - Several latent correctness bugs found while porting and pinned with upstream vectors: `sdk-core` `sqrt`, the FOT `Token` guard, zkSync address slicing, exact `Fraction` formatting (no float), `CurrencyAmount.ToExact()` overflow/format handling, `EncodeRouteToPath`, `Multicall` encoding, diff --git a/docs/PORTING.md b/docs/PORTING.md index 33f4157..712c8e4 100644 --- a/docs/PORTING.md +++ b/docs/PORTING.md @@ -79,6 +79,31 @@ None — all seven original `NotImplementedException` stubs are ported test-firs 3. Implement until green, matching numbers to the digit. 4. Update the table above. 5. PR into `main`. ## 6. Intentional divergences +- **Nullable-reference hardening (pre-1.0.0).** The public option/params types now carry explicit null + contracts derived from the upstream TypeScript interfaces: a required upstream field (`x: T`) is `required` + in C#, an optional one (`x?: T`) is nullable (`T?`). Applied across `NonfungiblePositionManager`, + `Staker`, `SelfPermit` and `Payments`. The library and test suite now build with **zero compiler warnings**. + Two root causes were fixed rather than suppressed: + - `CurrencyAmount.Wrapped()` / `.AsBaseCurrency()` were declared nullable but provably never return null + (every path constructs a value via the non-nullable `FromFractionalAmount`, and `BaseCurrency.Wrapped()` + returns a non-nullable `Token`). Tightening the return types to non-nullable removed all five downstream + flow warnings (`CS8601`/`CS8602`) at once and strengthens the contract for callers. + - `Route._midPrice` is a lazily-computed cache and is genuinely null until first access; it is now `Price<,>?`. +- **`PermitOptions` union modelled with a marker interface — fixes a broken API path.** Upstream declares + `PermitOptions = StandardPermitArguments | AllowedPermitArguments`. The port had typed + `Staker.SwapOptions.InputTokenPermit` as an **empty stub class** (`Staker.PermitOptions {}`) while + `SelfPermit.EncodePermit` took `object` and type-tested for the two argument interfaces. The result: the only + value assignable to `InputTokenPermit` satisfied neither test, so the V3 SwapRouter's input-token-permit path + **could only ever throw `"Invalid permit options"`** — and the valid argument types could not be assigned to it + at all. Upstream ships no test for this path, which is why the port inherited the gap. Fixed by introducing + `SelfPermit.IPermitOptions` (implemented by both argument interfaces), typing `EncodePermit` and every permit + option against it (`Staker.SwapOptions.InputTokenPermit`, `NonfungiblePositionManager.AddLiquidityOptions. + Token0Permit`/`Token1Permit`, `Router.SwapOptions.InputTokenPermit`/`SwapAndAddOptions.OutputTokenPermit` — + all previously `object?`), and deleting the empty stub. Pinned by `V3/SwapRouterPermitTests.cs`. +- **`PoolTests.BigNums_CorrectlyHandlesTwoBigIntegers` now awaits `GetInputAmount`.** Upstream calls + `pool.getInputAmount(outputAmount)` without awaiting ("if output is correct, function has succeeded"). In C# + an unawaited `Task` swallows exceptions, so the test would have passed even if `GetInputAmount` threw on the + very big numbers it is named for. Awaiting it makes the assertion real; it passes. - `CurrencyAmount.ToExact` now computes the exact decimal with `BigInteger` (integer part + zero-padded, trailing-trimmed fractional part), matching Decimal.js. The earlier `(decimal)` cast overflowed `System.Decimal` (~7.9e28) for large amounts; hardened test-first (`CurrencyAmountTests.cs`, incl. a diff --git a/src/UniswapSharp/Core/Entities/Fractions/CurrencyAmount.cs b/src/UniswapSharp/Core/Entities/Fractions/CurrencyAmount.cs index bec3fe9..8654fc5 100644 --- a/src/UniswapSharp/Core/Entities/Fractions/CurrencyAmount.cs +++ b/src/UniswapSharp/Core/Entities/Fractions/CurrencyAmount.cs @@ -97,26 +97,21 @@ public string ToExact(string format = "0.#############################") return integerPart.ToString(CultureInfo.InvariantCulture) + "." + fractional; } - public CurrencyAmount? AsBaseCurrency() => new(this.Currency, Numerator, Denominator) - { - - }; + public CurrencyAmount AsBaseCurrency() => new(this.Currency, Numerator, Denominator); - - public CurrencyAmount? Wrapped() + /// + /// The amount re-expressed against the currency's wrapped . + /// Never null: every path either reuses this instance or constructs a new amount. + /// + public CurrencyAmount Wrapped() { - if (Currency is Token) { var x = this as CurrencyAmount; - return x ?? FromFractionalAmount(Currency.Wrapped(), Numerator, Denominator); } - return FromFractionalAmount(Currency.Wrapped(), Numerator, Denominator); - - } public bool Equals(CurrencyAmount? other) diff --git a/src/UniswapSharp/Router/SwapRouter.cs b/src/UniswapSharp/Router/SwapRouter.cs index 546721e..3fff6f7 100644 --- a/src/UniswapSharp/Router/SwapRouter.cs +++ b/src/UniswapSharp/Router/SwapRouter.cs @@ -29,13 +29,15 @@ public class SwapOptions /// Either a deadline (epoch seconds, as BigInteger/int/string) or a previous block hash (0x… string). public object? DeadlineOrPreviousBlockhash { get; init; } - public object? InputTokenPermit { get; init; } + /// Accepts SelfPermit.StandardPermitArguments or SelfPermit.AllowedPermitArguments. + public V3.SelfPermit.IPermitOptions? InputTokenPermit { get; init; } public V3.Payments.IFeeOptions? Fee { get; init; } } public class SwapAndAddOptions : SwapOptions { - public object? OutputTokenPermit { get; init; } + /// Accepts SelfPermit.StandardPermitArguments or SelfPermit.AllowedPermitArguments. + public V3.SelfPermit.IPermitOptions? OutputTokenPermit { get; init; } } /// diff --git a/src/UniswapSharp/V3/Entities/Route.cs b/src/UniswapSharp/V3/Entities/Route.cs index 78034a0..4e4e69c 100644 --- a/src/UniswapSharp/V3/Entities/Route.cs +++ b/src/UniswapSharp/V3/Entities/Route.cs @@ -10,7 +10,8 @@ public class Route where TInput : BaseCurrency where TOutput : public TInput Input { get; } public TOutput Output { get; } - private Price _midPrice = null; + // Lazily computed and cached on first access — genuinely null until then. + private Price? _midPrice; public Route(List pools, TInput input, TOutput output) { diff --git a/src/UniswapSharp/V3/NonfungiblePositionManager.cs b/src/UniswapSharp/V3/NonfungiblePositionManager.cs index 2f4d9c6..65e728f 100644 --- a/src/UniswapSharp/V3/NonfungiblePositionManager.cs +++ b/src/UniswapSharp/V3/NonfungiblePositionManager.cs @@ -14,28 +14,29 @@ public class NonfungiblePositionManager { public class MethodParameters { - public string Calldata { get; set; } - public string Value { get; set; } + public required string Calldata { get; set; } + public required string Value { get; set; } } // Common options for adding liquidity (mint or increase). public abstract class AddLiquidityOptions { - public Percent SlippageTolerance { get; set; } + public required Percent SlippageTolerance { get; set; } public BigInteger Deadline { get; set; } // Whether to spend ether. If set, one of the pool tokens must be WETH. public NativeCurrency? UseNative { get; set; } - // Optional permit parameters for spending token0 / token1 (SelfPermit options). - public object? Token0Permit { get; set; } - public object? Token1Permit { get; set; } + // Optional permit parameters for spending token0 / token1. + // Accepts SelfPermit.StandardPermitArguments or SelfPermit.AllowedPermitArguments. + public SelfPermit.IPermitOptions? Token0Permit { get; set; } + public SelfPermit.IPermitOptions? Token1Permit { get; set; } } public class MintOptions : AddLiquidityOptions { // The account that should receive the minted NFT. - public string Recipient { get; set; } + public required string Recipient { get; set; } // Creates the pool if not initialized before mint. public bool CreatePool { get; set; } @@ -162,29 +163,29 @@ public static MethodParameters AddCallParameters(Position position, AddLiquidity public class CollectOptions { public BigInteger TokenId { get; set; } - public CurrencyAmount ExpectedCurrencyOwed0 { get; set; } - public CurrencyAmount ExpectedCurrencyOwed1 { get; set; } - public string Recipient { get; set; } + public required CurrencyAmount ExpectedCurrencyOwed0 { get; set; } + public required CurrencyAmount ExpectedCurrencyOwed1 { get; set; } + public required string Recipient { get; set; } } public class NFTPermitOptions { public byte V { get; set; } - public string R { get; set; } - public string S { get; set; } + public required string R { get; set; } + public required string S { get; set; } public BigInteger Deadline { get; set; } - public string Spender { get; set; } + public required string Spender { get; set; } } public class RemoveLiquidityOptions { public BigInteger TokenId { get; set; } - public Percent LiquidityPercentage { get; set; } - public Percent SlippageTolerance { get; set; } + public required Percent LiquidityPercentage { get; set; } + public required Percent SlippageTolerance { get; set; } public BigInteger Deadline { get; set; } public bool BurnToken { get; set; } public NFTPermitOptions? Permit { get; set; } - public CollectOptions CollectOptions { get; set; } + public required CollectOptions CollectOptions { get; set; } } private static List EncodeCollect(CollectOptions options) @@ -305,9 +306,10 @@ public static MethodParameters RemoveCallParameters(Position position, RemoveLiq public class SafeTransferOptions { - public string Sender { get; set; } - public string Recipient { get; set; } + public required string Sender { get; set; } + public required string Recipient { get; set; } public BigInteger TokenId { get; set; } + // upstream `data?: string` — optional. public string? Data { get; set; } } @@ -342,21 +344,21 @@ public static MethodParameters SafeTransferFromParameters(SafeTransferOptions op public class TypedDataField { - public string Name { get; set; } - public string Type { get; set; } + public required string Name { get; set; } + public required string Type { get; set; } } public class TypedDataDomain { - public string Name { get; set; } + public required string Name { get; set; } public int ChainId { get; set; } - public string Version { get; set; } - public string VerifyingContract { get; set; } + public required string Version { get; set; } + public required string VerifyingContract { get; set; } } public class NFTPermitValues { - public string Spender { get; set; } + public required string Spender { get; set; } public BigInteger TokenId { get; set; } public BigInteger Deadline { get; set; } public BigInteger Nonce { get; set; } @@ -364,9 +366,9 @@ public class NFTPermitValues public class NFTPermitData { - public TypedDataDomain Domain { get; set; } - public Dictionary> Types { get; set; } - public NFTPermitValues Values { get; set; } + public required TypedDataDomain Domain { get; set; } + public required Dictionary> Types { get; set; } + public required NFTPermitValues Values { get; set; } } private static Dictionary> NftPermitTypes() => new() diff --git a/src/UniswapSharp/V3/Payments.cs b/src/UniswapSharp/V3/Payments.cs index 1ce88d0..1384660 100644 --- a/src/UniswapSharp/V3/Payments.cs +++ b/src/UniswapSharp/V3/Payments.cs @@ -75,7 +75,7 @@ public interface IFeeOptions public class FeeOptions : IFeeOptions { - public Percent Fee { get; set; } - public string Recipient { get; set; } + public required Percent Fee { get; set; } + public required string Recipient { get; set; } } } diff --git a/src/UniswapSharp/V3/SelfPermit.cs b/src/UniswapSharp/V3/SelfPermit.cs index 051bebd..9f55df8 100644 --- a/src/UniswapSharp/V3/SelfPermit.cs +++ b/src/UniswapSharp/V3/SelfPermit.cs @@ -8,7 +8,12 @@ namespace UniswapSharp.V3; public static class SelfPermit { - public static string EncodePermit(Token token, object options) + /// + /// Marker for the upstream union PermitOptions = StandardPermitArguments | AllowedPermitArguments. + /// + public interface IPermitOptions; + + public static string EncodePermit(Token token, IPermitOptions options) { if (options is IAllowedPermitArguments allowedOptions) { @@ -36,7 +41,7 @@ public static string EncodePermit(Token token, object options) } } - public interface IAllowedPermitArguments + public interface IAllowedPermitArguments : IPermitOptions { byte V { get; } string R { get; } @@ -45,7 +50,7 @@ public interface IAllowedPermitArguments BigInteger Expiry { get; } } - public interface IStandardPermitArguments + public interface IStandardPermitArguments : IPermitOptions { byte V { get; } string R { get; } @@ -57,8 +62,8 @@ public interface IStandardPermitArguments public class AllowedPermitArguments : IAllowedPermitArguments { public byte V { get; set; } - public string R { get; set; } - public string S { get; set; } + public required string R { get; set; } + public required string S { get; set; } public BigInteger Nonce { get; set; } public BigInteger Expiry { get; set; } } @@ -66,8 +71,8 @@ public class AllowedPermitArguments : IAllowedPermitArguments public class StandardPermitArguments : IStandardPermitArguments { public byte V { get; set; } - public string R { get; set; } - public string S { get; set; } + public required string R { get; set; } + public required string S { get; set; } public BigInteger Amount { get; set; } public BigInteger Deadline { get; set; } } diff --git a/src/UniswapSharp/V3/Staker.cs b/src/UniswapSharp/V3/Staker.cs index d566c6a..d7386ac 100644 --- a/src/UniswapSharp/V3/Staker.cs +++ b/src/UniswapSharp/V3/Staker.cs @@ -131,19 +131,20 @@ public static string EncodeDeposit(IncentiveKey[] incentiveKeys) public class FullWithdrawOptions : IClaimOptions, IWithdrawOptions { public BigInteger TokenId { get; set; } - public string Recipient { get; set; } + public required string Recipient { get; set; } public BigInteger? Amount { get; set; } - public string Owner { get; set; } - public string Data { get; set; } + public required string Owner { get; set; } + // upstream `data?: string` — optional. + public string? Data { get; set; } } public class IncentiveKey { - public Token RewardToken { get; set; } - public Pool Pool { get; set; } + public required Token RewardToken { get; set; } + public required Pool Pool { get; set; } public BigInteger StartTime { get; set; } public BigInteger EndTime { get; set; } - public string Refundee { get; set; } + public required string Refundee { get; set; } } public interface IClaimOptions @@ -156,28 +157,25 @@ public interface IClaimOptions public class ClaimOptions : IClaimOptions { public BigInteger TokenId { get; set; } - public string Recipient { get; set; } + public required string Recipient { get; set; } public BigInteger? Amount { get; set; } } public interface IWithdrawOptions { public string Owner { get; set; } - public string Data { get; set; } - } - - public class PermitOptions - { - // Implement PermitOptions + public string? Data { get; set; } } public class SwapOptions { - public Percent SlippageTolerance { get; set; } - public string Recipient { get; set; } + public required Percent SlippageTolerance { get; set; } + public required string Recipient { get; set; } public BigInteger Deadline { get; set; } - public PermitOptions InputTokenPermit { get; set; } + // upstream `inputTokenPermit?: PermitOptions` / `fee?: FeeOptions` — optional. + // Accepts SelfPermit.StandardPermitArguments or SelfPermit.AllowedPermitArguments. + public SelfPermit.IPermitOptions? InputTokenPermit { get; set; } public BigInteger? SqrtPriceLimitX96 { get; set; } - public Payments.FeeOptions Fee { get; set; } + public Payments.FeeOptions? Fee { get; set; } } } diff --git a/test/UniswapSharp.Testing/V3/ConstantsTests.cs b/test/UniswapSharp.Testing/V3/ConstantsTests.cs index 3c595b1..75bcf06 100644 --- a/test/UniswapSharp.Testing/V3/ConstantsTests.cs +++ b/test/UniswapSharp.Testing/V3/ConstantsTests.cs @@ -18,8 +18,9 @@ public void InitCodeHash_MatchesComputedBytecodeHash() var computedInitCodeHash = Sha3Keccack.Current.CalculateHash(bytecodeBytes); var computedInitCodeHashHex = "0x" + BitConverter.ToString(computedInitCodeHash).Replace("-", "").ToLowerInvariant(); - // Assert that the computed hash matches the constant - Assert.Equal(Constants.POOL_INIT_CODE_HASH, computedInitCodeHashHex); + // Assert that the computed hash matches the supported API (which shares the same + // source-of-truth constant as the obsolete POOL_INIT_CODE_HASH). + Assert.Equal(Constants.PoolInitCodeHash(), computedInitCodeHashHex); } diff --git a/test/UniswapSharp.Testing/V3/Entities/PoolTests.cs b/test/UniswapSharp.Testing/V3/Entities/PoolTests.cs index 8c2424c..0fbb20d 100644 --- a/test/UniswapSharp.Testing/V3/Entities/PoolTests.cs +++ b/test/UniswapSharp.Testing/V3/Entities/PoolTests.cs @@ -282,7 +282,9 @@ public async Task BigNums_CorrectlyHandlesTwoBigIntegers() var inputAmount = CurrencyAmount.FromRawAmount(USDC, 100); var outputAmount = (await pool.GetOutputAmount(inputAmount)).outputAmount; - pool.GetInputAmount(outputAmount); + // Upstream calls this without awaiting; in C# an unawaited Task swallows exceptions, which + // would let this test pass even if GetInputAmount threw on the big numbers it is named for. + await pool.GetInputAmount(outputAmount); Assert.True(outputAmount.Currency.Equals(DAI)); } } diff --git a/test/UniswapSharp.Testing/V3/SwapRouterPermitTests.cs b/test/UniswapSharp.Testing/V3/SwapRouterPermitTests.cs new file mode 100644 index 0000000..7f29309 --- /dev/null +++ b/test/UniswapSharp.Testing/V3/SwapRouterPermitTests.cs @@ -0,0 +1,58 @@ +using System.Numerics; +using UniswapSharp.Core; +using UniswapSharp.Core.Entities; +using UniswapSharp.Core.Entities.Fractions; +using UniswapSharp.V3; +using UniswapSharp.V3.Entities; +using UniswapSharp.V3.Utils; +using static UniswapSharp.V3.Constants; + +namespace UniswapSharp.Testing.V3; + +// Regression: SwapOptions.InputTokenPermit must be able to carry a real permit +// (upstream `inputTokenPermit?: PermitOptions = StandardPermitArguments | AllowedPermitArguments`) +// and SwapRouter must prepend the selfPermit calldata. Upstream ships no test for this path. +public class SwapRouterPermitTests +{ + private static readonly Token token0 = new(1, "0x0000000000000000000000000000000000000001", 18, "t0", "token0"); + private static readonly Token token1 = new(1, "0x0000000000000000000000000000000000000002", 18, "t1", "token1"); + + private static Pool MakePool(Token a, Token b) => new( + a, b, FeeAmount.MEDIUM, EncodeSqrtRatioX96.Encode(1, 1), BigInteger.Parse("1000000000000000000"), + 0, new List + { + new(NearestUsableTick.Find(TickMath.MIN_TICK, TICK_SPACINGS[FeeAmount.MEDIUM]), BigInteger.Parse("1000000000000000000"), BigInteger.Parse("1000000000000000000")), + new(NearestUsableTick.Find(TickMath.MAX_TICK, TICK_SPACINGS[FeeAmount.MEDIUM]), BigInteger.Parse("-1000000000000000000"), BigInteger.Parse("1000000000000000000")), + }); + + [Fact] + public async Task SwapCallParameters_WithStandardInputTokenPermit_PrependsSelfPermitCalldata() + { + var pool = MakePool(token0, token1); + var trade = await Trade.FromRoute( + new Route(new List { pool }, token0, token1), + CurrencyAmount.FromRawAmount(token0, 100), + TradeType.EXACT_INPUT); + + var permit = new SelfPermit.StandardPermitArguments + { + V = 1, + R = "0x0000000000000000000000000000000000000000000000000000000000000001", + S = "0x0000000000000000000000000000000000000000000000000000000000000002", + Amount = 100, + Deadline = 123, + }; + + var result = SwapRouter.SwapCallParameters(trade, new Staker.SwapOptions + { + SlippageTolerance = new Percent(1, 100), + Recipient = "0x0000000000000000000000000000000000000003", + Deadline = 123, + InputTokenPermit = permit, + }); + + // The permit must be encoded and included, matching SelfPermit's (vector-tested) encoding. + string expectedPermit = SelfPermit.EncodePermit(token0, permit); + Assert.Contains(expectedPermit.Substring(2), result.Calldata); + } +}