diff --git a/CHANGES.md b/CHANGES.md index 64d719be72f..6b08a7a320e 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -10,6 +10,12 @@ To be released. ### Backward-incompatible API changes + - (Libplanet.Types) Changed `Currency` from a `struct` to a `class`. + [[#3888]] + - (Libplanet.Types) Changed `Currency()` to throw an `ArgumentException` + if an argument for `ticker` has leading and/or trailing white spaces. + [[#3888]] + ### Backward-incompatible network protocol changes ### Backward-incompatible storage format changes @@ -20,10 +26,15 @@ To be released. ### Bug fixes + - (Libplanet.Types) Fixed several bugs where an invalid `Currency` object + could be created through deserialization. [[#3888]] + ### Dependencies ### CLI tools +[#3888]: https://github.com/planetarium/libplanet/pull/3888 + Version 5.2.2 ------------- diff --git a/src/Libplanet.Types/Assets/Currency.cs b/src/Libplanet.Types/Assets/Currency.cs index 97d7e4cd670..8623cfc26c2 100644 --- a/src/Libplanet.Types/Assets/Currency.cs +++ b/src/Libplanet.Types/Assets/Currency.cs @@ -1,5 +1,4 @@ using System; -using System.Collections.Generic; using System.Collections.Immutable; using System.Diagnostics.CodeAnalysis; using System.Diagnostics.Contracts; @@ -13,7 +12,6 @@ using Bencodex; using Bencodex.Types; using Libplanet.Common; -using Libplanet.Common.Serialization; using Libplanet.Crypto; namespace Libplanet.Types.Assets @@ -49,47 +47,14 @@ namespace Libplanet.Types.Assets /// /// [Serializable] - public readonly struct Currency : IEquatable, ISerializable + public sealed class Currency : IEquatable, ISerializable { - /// - /// The ticker symbol, e.g., "USD". - /// - [JsonInclude] - public readonly string Ticker; - - /// - /// The number of digits to treat as minor - /// units (i.e., exponent). - /// - [JsonInclude] - public readonly byte DecimalPlaces; - - /// - /// The es who can mint the currency. - /// If this is anyone can mint the currency. - /// - /// - /// Unlike , an empty set means no one can mint the currency. - /// - /// - [JsonInclude] - public readonly IImmutableSet
? Minters; - - /// - /// The deterministic hash derived from other fields. - /// - [JsonInclude] - public readonly HashDigest Hash; - - /// - /// Whether the total supply of this instance of is trackable. - /// - [JsonInclude] - public readonly bool TotalSupplyTrackable; + private static readonly Codec _codec = new Codec(); private readonly (BigInteger Major, BigInteger Minor)? _maximumSupply; + private HashDigest? _hash; + /// /// Deserializes a type from a Bencodex value. /// @@ -97,153 +62,13 @@ namespace Libplanet.Types.Assets /// method. /// public Currency(IValue serialized) + : this(serialized is Dictionary dictionary + ? dictionary + : throw new ArgumentException( + $"Given {nameof(serialized)} must be of type " + + $"{typeof(Dictionary)}: {serialized.GetType()}", + nameof(serialized))) { - BigInteger? maximumSupplyMajor = null, maximumSupplyMinor = null; - - if (!(serialized is Dictionary d)) - { - throw new ArgumentException("Expected a Bencodex dictionary.", nameof(serialized)); - } - - if (!(d.ContainsKey("ticker") && d["ticker"] is Text ticker)) - { - throw new ArgumentException( - "Expected a text field named \"ticker\".", - nameof(serialized) - ); - } - - byte? nullableDecimals = null; - if (d.ContainsKey("decimals") && d["decimals"] is Integer decimals) - { - nullableDecimals = (byte)(long)decimals; - } - - if (d.ContainsKey("decimalPlaces") && d["decimalPlaces"] is Binary decimalPlacesBinary) - { - nullableDecimals = decimalPlacesBinary.ByteArray[0]; - } - - if (!(nullableDecimals is { } decimalPlaces)) - { - throw new ArgumentException( - "Expected a byte field named \"decimalPlaces\" or \"decimals\".", - nameof(serialized) - ); - } - - if (!d.ContainsKey("minters") || - !(d["minters"] is { } minters) || - !(minters is Null || minters is List)) - { - throw new ArgumentException( - "Expected a nullable list field named \"minters\".", - nameof(serialized) - ); - } - - TotalSupplyTrackable = false; - if (d.ContainsKey("totalSupplyTrackable")) - { - if (d["totalSupplyTrackable"] is Bencodex.Types.Boolean totalSupplyTrackable - && totalSupplyTrackable) - { - TotalSupplyTrackable = totalSupplyTrackable; - } - else - { - throw new ArgumentException( - "Field \"totalSupplyTrackable\" must have a boolean value of true" - + " if it exists.", - nameof(serialized)); - } - } - - if (d.ContainsKey("maximumSupplyMajor")) - { - if (d["maximumSupplyMajor"] is Integer maximumSupplyMajorValue - && maximumSupplyMajorValue >= 0) - { - maximumSupplyMajor = maximumSupplyMajorValue.Value; - } - else - { - throw new ArgumentException( - "Field \"maximumSupplyMajor\" must be of non-negative integer type" - + " if it exists.", - nameof(serialized) - ); - } - } - - if (d.ContainsKey("maximumSupplyMinor")) - { - if (d["maximumSupplyMinor"] is Integer maximumSupplyMinorValue - && maximumSupplyMinorValue >= 0) - { - maximumSupplyMinor = maximumSupplyMinorValue.Value; - } - else - { - throw new ArgumentException( - "Field \"maximumSupplyMinor\" must be of non-negative integer type" - + " if it exists.", - nameof(serialized) - ); - } - } - - if (maximumSupplyMajor is { } && maximumSupplyMinor is { }) - { - if (!TotalSupplyTrackable) - { - throw new ArgumentException( - $"Maximum supply is not available for legacy untracked currencies.", - nameof(serialized)); - } - - _maximumSupply = (maximumSupplyMajor.Value, maximumSupplyMinor.Value); - } - else if (maximumSupplyMajor is null && maximumSupplyMinor is null) - { - _maximumSupply = null; - } - else - { - throw new ArgumentException( - "Both \"maximumSupplyMajor\" and \"maximumSupplyMinor\" must be " - + " omitted or be non-negative integers.", - nameof(serialized) - ); - } - - Ticker = ticker; - DecimalPlaces = decimalPlaces; - - if (_maximumSupply is var (_, minor) && minor > 0 && - Math.Floor(BigInteger.Log10(minor)) >= DecimalPlaces) - { - var msg = $"The given minor unit {minor} of the maximum supply value is too" - + $" big for the given decimal places {DecimalPlaces}."; - throw new ArgumentException(msg, nameof(minor)); - } - - if (minters is List l) - { - Minters = l.Select( - m => m is Binary b - ? new Address(b.ByteArray) - : throw new ArgumentException( - "Expected \"minters\" to be a list of binary arrays.", - nameof(serialized)) - ).ToImmutableHashSet(); - } - else - { - Minters = null; - } - - Hash = GetHash(Minters, Ticker, DecimalPlaces, _maximumSupply, TotalSupplyTrackable); } /// @@ -268,11 +93,10 @@ public Currency( maximumSupply is { } v ? (v.MajorUnit, v.MinorUnit) : ((BigInteger, BigInteger)?)null, - minters - ) + minters, + totalSupplyTrackable) #pragma warning restore SA1118 { - TotalSupplyTrackable = totalSupplyTrackable; HashDigest expectedHash = GetHash( Minters, Ticker, DecimalPlaces, _maximumSupply, TotalSupplyTrackable); if (!expectedHash.Equals(hash)) @@ -285,124 +109,10 @@ maximumSupply is { } v $" maximumSupply: {MaximumSupply?.ToString() ?? "N/A"}"; throw new JsonException(msg); } - - Hash = hash; - } - - private Currency(SerializationInfo info, StreamingContext context) - { - Ticker = info.GetValue(nameof(Ticker)); - DecimalPlaces = info.GetValue(nameof(DecimalPlaces)); - - if (info.TryGetValue(nameof(Minters), out List minters)) - { - Minters = minters.Select(m => new Address(m)).ToImmutableHashSet(); - } - else - { - Minters = default; - } - - TotalSupplyTrackable = false; - if (info.TryGetValue(nameof(TotalSupplyTrackable), out bool totalSupplyTrackable)) - { - if (!totalSupplyTrackable) - { - throw new ArgumentException( - $"{nameof(TotalSupplyTrackable)} must be true if it exists in the" - + "SerializationInfo.", - nameof(TotalSupplyTrackable)); - } - - TotalSupplyTrackable = totalSupplyTrackable; - } - - if (info.TryGetValue(nameof(MaximumSupply), out (BigInteger, BigInteger) maximumSupply)) - { - if (!TotalSupplyTrackable) - { - throw new ArgumentException( - $"Maximum supply is not available for legacy untracked currencies.", - nameof(info)); - } - - _maximumSupply = maximumSupply; - } - else - { - _maximumSupply = null; - } - - if (_maximumSupply is var (major, minor)) - { - if (major < 0 || minor < 0) - { - var msg = $"Both the major ({major}) and minor ({minor}) units of" - + $" {nameof(maximumSupply)} must not be a negative number."; - throw new ArgumentException(msg, nameof(maximumSupply)); - } - - if (minor > 0 && Math.Floor(BigInteger.Log10(minor)) >= DecimalPlaces) - { - var msg = $"The given minor unit {minor} of the maximum supply value is too" - + $" big for the given decimal places {DecimalPlaces}."; - throw new ArgumentException(msg, nameof(minor)); - } - } - - Hash = GetHash(Minters, Ticker, DecimalPlaces, _maximumSupply, TotalSupplyTrackable); } /// - /// Private implementation to create a capped instance of or - /// a deserialized instance. - /// - /// The ticker symbol, e.g., "USD". - /// The number of digits to treat as minor - /// units (i.e., exponent). - /// The uppermost quantity of currency allowed to exist. For - /// example, a parameter of (123, 45) means that the - /// token of the currency can be minted up to 123.45. See also - /// field which corresponds to this. - /// The es who can mint the currency. - /// See also field which corresponds to this. - /// Thrown when the given - /// is an empty string, or when either the Major or the Minor values of - /// is a negative number, or when the given Minor unit for - /// the is too big for the given - /// . - private Currency( - string ticker, - byte decimalPlaces, - (BigInteger Major, BigInteger Minor)? maximumSupply, - IImmutableSet
? minters) - : this(ticker, decimalPlaces, minters, true) - { - if (maximumSupply is var (major, minor)) - { - if (major < 0 || minor < 0) - { - var msg = $"Both the major ({major}) and minor ({minor}) units of" - + $" {nameof(maximumSupply)} must not be a negative number."; - throw new ArgumentException(msg, nameof(maximumSupply)); - } - - if (minor > 0 && Math.Floor(BigInteger.Log10(minor)) >= decimalPlaces) - { - var msg = $"The given minor unit {minor} of the maximum supply value is too" - + $" big for the given decimal places {decimalPlaces}."; - throw new ArgumentException(msg, nameof(minor)); - } - - _maximumSupply = maximumSupply; - } - - Hash = GetHash(Minters, Ticker, DecimalPlaces, _maximumSupply, TotalSupplyTrackable); - } - - /// - /// Private implementation to create a general instance of . + /// Private implementation to create an instance of . /// /// The ticker symbol, e.g., "USD". /// The number of digits to treat as ? minters, bool totalSupplyTrackable) { - ticker = ticker.Trim(); - - if (string.IsNullOrEmpty(ticker)) - { - throw new ArgumentException( - "Currency ticker symbol cannot be empty.", - nameof(ticker) - ); - } - + ValidateArguments(ticker, decimalPlaces, maximumSupply, minters, totalSupplyTrackable); Ticker = ticker; Minters = minters; DecimalPlaces = decimalPlaces; - _maximumSupply = null; + _maximumSupply = maximumSupply; TotalSupplyTrackable = totalSupplyTrackable; - Hash = GetHash(Minters, Ticker, DecimalPlaces, _maximumSupply, TotalSupplyTrackable); } + private Currency(Dictionary dictionary) + : this( + ExtractTicker(dictionary), + ExtractDecimalPlaces(dictionary), + ExtractMaximumSupply(dictionary), + ExtractMinters(dictionary), + ExtractTotalSupplyTrackable(dictionary)) + { + } + + private Currency(SerializationInfo info, StreamingContext context) + : this(_codec.Decode((byte[])info.GetValue("ByteEncoded", typeof(byte[]))!)) + { + } + + /// + /// The ticker symbol, e.g., "USD". + /// + [JsonInclude] + public string Ticker { get; } + + /// + /// The number of digits to treat as minor + /// units (i.e., exponent). + /// + [JsonInclude] + public byte DecimalPlaces { get; } + + /// + /// The es who can mint the currency. + /// If this is anyone can mint the currency. + /// + /// + /// Unlike , an empty set means no one can mint the currency. + /// + /// + [JsonInclude] + public IImmutableSet
? Minters { get; } + + /// + /// The deterministic hash derived from other fields. + /// + [JsonInclude] + public HashDigest Hash => _hash ??= + GetHash(Minters, Ticker, DecimalPlaces, _maximumSupply, TotalSupplyTrackable); + + /// + /// Whether the total supply of this instance of is trackable. + /// + [JsonInclude] + public bool TotalSupplyTrackable { get; } + /// /// The uppermost quantity of currency allowed to exist. /// means unlimited supply. @@ -509,7 +263,7 @@ public static Currency Capped( byte decimalPlaces, (BigInteger Major, BigInteger Minor) maximumSupply, IImmutableSet
? minters) => - new Currency(ticker, decimalPlaces, maximumSupply, minters); + new Currency(ticker, decimalPlaces, maximumSupply, minters, true); /// /// Define a with a maximum supply limit. @@ -557,7 +311,7 @@ public static Currency Uncapped( string ticker, byte decimalPlaces, IImmutableSet
? minters) => - new Currency(ticker, decimalPlaces, minters, true); + new Currency(ticker, decimalPlaces, null, minters, true); /// /// Define a without a maximum supply limit. @@ -601,7 +355,7 @@ public static Currency Legacy( string ticker, byte decimalPlaces, IImmutableSet
? minters) => - new Currency(ticker, decimalPlaces, minters, false); + new Currency(ticker, decimalPlaces, null, minters, false); /// /// OBSOLETE! DO NOT USE.

(unless you are upgrading your project from an old @@ -644,23 +398,7 @@ public static Currency Legacy( /// void ISerializable.GetObjectData(SerializationInfo info, StreamingContext context) { - info.AddValue(nameof(Ticker), Ticker); - info.AddValue(nameof(DecimalPlaces), DecimalPlaces); - - if (Minters is IImmutableSet
minters) - { - info.AddValue(nameof(Minters), minters.Select(m => m.ToByteArray()).ToList()); - } - - if (_maximumSupply is { } maximumSupply) - { - info.AddValue(nameof(MaximumSupply), maximumSupply); - } - - if (TotalSupplyTrackable) - { - info.AddValue(nameof(TotalSupplyTrackable), TotalSupplyTrackable); - } + info.AddValue("ByteEncoded", _codec.Encode(Serialize())); } /// @@ -678,8 +416,8 @@ public override bool Equals(object? obj) => /// [Pure] - public bool Equals(Currency other) => - Hash.Equals(other.Hash); + public bool Equals(Currency? other) => + other is Currency currency && Hash.Equals(currency.Hash); /// /// Serializes the currency into a Bencodex value. @@ -700,18 +438,129 @@ public IValue Serialize() serialized = serialized.Add("totalSupplyTrackable", true); if (MaximumSupply is { } maximumSupply) { - serialized = serialized.Add( - "maximumSupplyMajor", - (IValue)new Integer(maximumSupply.MajorUnit) - ).Add( - "maximumSupplyMinor", - (IValue)new Integer(maximumSupply.MinorUnit)); + serialized = serialized + .Add("maximumSupplyMajor", new Integer(maximumSupply.MajorUnit)) + .Add("maximumSupplyMinor", new Integer(maximumSupply.MinorUnit)); } } return serialized; } + private static string ExtractTicker(Dictionary dict) => + ((Text)dict["ticker"]).Value; + + private static byte ExtractDecimalPlaces(Dictionary dict) + { + if (!(dict.ContainsKey("decimals") ^ dict.ContainsKey("decimalPlaces"))) + { + throw new ArgumentException( + "Serialized value should contain exactly one of the two keys " + + "\"decimals\" and \"decimalPlaces\"."); + } + + return dict.ContainsKey("decimals") + ? (byte)(long)((Integer)dict["decimals"]) + : ((Binary)dict["decimalPlaces"]).ByteArray[0]; + } + + private static (BigInteger Major, BigInteger Minor)? ExtractMaximumSupply( + Dictionary dict) + { + if (dict.ContainsKey("maximumSupplyMajor") ^ dict.ContainsKey("maximumSupplyMinor")) + { + throw new ArgumentException( + "Serialized value should either contain both or non of the two keys " + + "\"maximumSupplyMajor\" and \"maximumSupplyMinor\"."); + } + + if (dict.ContainsKey("maximumSupplyMajor")) + { + return ( + ((Integer)dict["maximumSupplyMajor"]).Value, + ((Integer)dict["maximumSupplyMinor"]).Value); + } + else + { + return null; + } + } + + private static IImmutableSet
? ExtractMinters(Dictionary dict) + { + switch (dict["minters"]) + { + case List list: + return list + .Select(minter => new Address(((Binary)minter).ByteArray)) + .ToImmutableHashSet(); + case Null _: + return null; + default: + throw new ArgumentException( + $"The value associated with key \"minters\" should be either " + + $"{nameof(List)} or {nameof(Null)}."); + } + } + + private static bool ExtractTotalSupplyTrackable(Dictionary dict) => + dict.ContainsKey("totalSupplyTrackable") + ? ((Bencodex.Types.Boolean)dict["totalSupplyTrackable"]).Value + ? true + : throw new ArgumentException( + $"The value associated with key \"totalSupplyTrackable\" cannot be false.") + : false; + + private static void ValidateArguments( + string ticker, + byte decimalPlaces, + (BigInteger Major, BigInteger Minor)? maximumSupply, + IImmutableSet
? minters, + bool totalSupplyTrackable) + { + if (!ticker.Equals(ticker.Trim())) + { + throw new ArgumentException( + $"Currency ticker should not have any leading and/or trailing white spaces.", + nameof(ticker)); + } + + if (string.IsNullOrEmpty(ticker)) + { + throw new ArgumentException( + "Currency ticker symbol cannot be empty.", + nameof(ticker)); + } + + if (maximumSupply is { } && !totalSupplyTrackable) + { + throw new ArgumentException( + $"Either {nameof(totalSupplyTrackable)} {totalSupplyTrackable} and " + + $"{nameof(maximumSupply)} {maximumSupply} " + + $"should be both null or both non-null.", + nameof(totalSupplyTrackable)); + } + + if (maximumSupply is { } supply) + { + if (supply.Major < 0 || supply.Minor < 0) + { + var msg = $"Both the major value {supply.Major} and " + + $"minor value {supply.Minor} of {nameof(maximumSupply)} " + + $"cannot be a negative number."; + throw new ArgumentException(msg, nameof(maximumSupply)); + } + + if (supply.Major > 0 && + Math.Floor(BigInteger.Log10(supply.Minor)) >= decimalPlaces) + { + var msg = $"Given minor value {supply.Minor} of {nameof(maximumSupply)} is " + + $"too big for given decimal places {decimalPlaces}."; + throw new ArgumentException(msg, nameof(maximumSupply)); + } + } + } + private static SHA1 GetSHA1() { #if NETSTANDARD2_0_OR_GREATER || NETCOREAPP3_1 @@ -752,7 +601,8 @@ bool totalSupplyTrackable if (maximumSupply is var (major, minor)) { - serialized = serialized.Add("maximumSupplyMajor", new Integer(major)) + serialized = serialized + .Add("maximumSupplyMajor", new Integer(major)) .Add("maximumSupplyMinor", new Integer(minor)); } diff --git a/test/Libplanet.Tests/Assets/CurrencyTest.cs b/test/Libplanet.Tests/Assets/CurrencyTest.cs index b48902bd9ad..cf5cf6e5bae 100644 --- a/test/Libplanet.Tests/Assets/CurrencyTest.cs +++ b/test/Libplanet.Tests/Assets/CurrencyTest.cs @@ -33,7 +33,7 @@ public void Constructor() Assert.False(foo.TotalSupplyTrackable); var bar = Currency.Capped( - "\t BAR \n", + "BAR", 0, (100, 0), ImmutableHashSet.Create(AddressA, AddressB)); @@ -65,23 +65,19 @@ public void Constructor() Assert.True(qux.TotalSupplyTrackable); Assert.Throws(() => - Currency.Uncapped(string.Empty, 0, ImmutableHashSet
.Empty) - ); + Currency.Uncapped(string.Empty, 0, ImmutableHashSet
.Empty)); Assert.Throws(() => - Currency.Uncapped(" \n", 1, ImmutableHashSet
.Empty) - ); + Currency.Uncapped(" \n", 1, ImmutableHashSet
.Empty)); Assert.Throws(() => - Currency.Capped("TEST", 0, (100, 1), ImmutableHashSet
.Empty) - ); + Currency.Uncapped("\t Test \n", 3, null)); Assert.Throws(() => - Currency.Capped("TEST", 1, (-100, 1), ImmutableHashSet
.Empty) - ); + Currency.Capped("TEST", 0, (100, 1), ImmutableHashSet
.Empty)); Assert.Throws(() => - Currency.Capped("TEST", 1, (100, -1), ImmutableHashSet
.Empty) - ); + Currency.Capped("TEST", 1, (-100, 1), ImmutableHashSet
.Empty)); Assert.Throws(() => - Currency.Capped("TEST", 1, (-100, -1), ImmutableHashSet
.Empty) - ); + Currency.Capped("TEST", 1, (100, -1), ImmutableHashSet
.Empty)); + Assert.Throws(() => + Currency.Capped("TEST", 1, (-100, -1), ImmutableHashSet
.Empty)); } [Fact]