From 214a350e005dbc4fbd0d906b42567d2e22ca1559 Mon Sep 17 00:00:00 2001 From: Say Cheong Date: Fri, 10 May 2024 13:22:31 +0900 Subject: [PATCH 1/4] Reorganized code for transfer asset --- Libplanet.Action/State/CurrencyAccount.cs | 83 +++++++++++++++------- Libplanet.Action/State/IWorldExtensions.cs | 15 ++-- 2 files changed, 69 insertions(+), 29 deletions(-) diff --git a/Libplanet.Action/State/CurrencyAccount.cs b/Libplanet.Action/State/CurrencyAccount.cs index ecdef8d0c51..dd43816c2b0 100644 --- a/Libplanet.Action/State/CurrencyAccount.cs +++ b/Libplanet.Action/State/CurrencyAccount.cs @@ -111,7 +111,6 @@ public CurrencyAccount BurnAsset( } public CurrencyAccount TransferAsset( - IActionContext context, Address sender, Address recipient, FungibleAssetValue value) @@ -125,8 +124,27 @@ public CurrencyAccount TransferAsset( } return WorldVersion >= BlockMetadata.CurrencyAccountProtocolVersion - ? TransferRawAssetV7(context, sender, recipient, value.RawValue) - : TransferRawAssetV0(context, sender, recipient, value.RawValue); + ? TransferRawAssetV7(sender, recipient, value.RawValue) + : TransferRawAssetV1(sender, recipient, value.RawValue); + } + + [Obsolete( + "Should not be used unless to specifically keep backwards compatibility " + + "for IActions that's been used when block protocol version was 0.")] + public CurrencyAccount TransferAssetV0( + Address sender, + Address recipient, + FungibleAssetValue value) + { + CheckCurrency(value.Currency); + if (value.Sign <= 0) + { + throw new ArgumentOutOfRangeException( + nameof(value), + $"The amount to mint, burn, or transfer must be greater than zero: {value}"); + } + + return TransferRawAssetV0(sender, recipient, value.RawValue); } public IAccount AsAccount() @@ -260,7 +278,6 @@ private CurrencyAccount BurnRawAssetV7( } private CurrencyAccount TransferRawAssetV7( - IActionContext context, Address sender, Address recipient, BigInteger rawValue) @@ -289,8 +306,7 @@ private CurrencyAccount TransferRawAssetV7( return currencyAccount; } - private CurrencyAccount TransferRawAssetV0( - IActionContext context, + private CurrencyAccount TransferRawAssetV1( Address sender, Address recipient, BigInteger rawValue) @@ -311,29 +327,46 @@ private CurrencyAccount TransferRawAssetV0( // NOTE: For backward compatibility with the bugged behavior before // protocol version 1. - if (context.BlockProtocolVersion == 0) - { - BigInteger prevRecipientBalanceRawValue = - currencyAccount.GetRawBalanceV0(recipient); - currencyAccount = currencyAccount.WriteRawBalanceV0( - sender, - prevSenderBalanceRawValue - rawValue); - currencyAccount = currencyAccount.WriteRawBalanceV0( - recipient, - prevRecipientBalanceRawValue + rawValue); - } - else + currencyAccount = currencyAccount.WriteRawBalanceV0( + sender, + prevSenderBalanceRawValue - rawValue); + BigInteger prevRecipientBalanceRawValue = + currencyAccount.GetRawBalanceV0(recipient); + currencyAccount = currencyAccount.WriteRawBalanceV0( + recipient, + prevRecipientBalanceRawValue + rawValue); + return currencyAccount; + } + + private CurrencyAccount TransferRawAssetV0( + Address sender, + Address recipient, + BigInteger rawValue) + { + CurrencyAccount currencyAccount = this; + BigInteger prevSenderBalanceRawValue = currencyAccount.GetRawBalanceV0(sender); + if (prevSenderBalanceRawValue - rawValue < 0) { - currencyAccount = currencyAccount.WriteRawBalanceV0( + FungibleAssetValue prevSenderBalance = + FungibleAssetValue.FromRawValue(Currency, prevSenderBalanceRawValue); + FungibleAssetValue value = FungibleAssetValue.FromRawValue(Currency, rawValue); + throw new InsufficientBalanceException( + $"Cannot burn or transfer {value} from {sender} as the current balance " + + $"of {sender} is {prevSenderBalance}.", sender, - prevSenderBalanceRawValue - rawValue); - BigInteger prevRecipientBalanceRawValue = - currencyAccount.GetRawBalanceV0(recipient); - currencyAccount = currencyAccount.WriteRawBalanceV0( - recipient, - prevRecipientBalanceRawValue + rawValue); + prevSenderBalance); } + // NOTE: For backward compatibility with the bugged behavior before + // protocol version 1. + BigInteger prevRecipientBalanceRawValue = + currencyAccount.GetRawBalanceV0(recipient); + currencyAccount = currencyAccount.WriteRawBalanceV0( + sender, + prevSenderBalanceRawValue - rawValue); + currencyAccount = currencyAccount.WriteRawBalanceV0( + recipient, + prevRecipientBalanceRawValue + rawValue); return currencyAccount; } diff --git a/Libplanet.Action/State/IWorldExtensions.cs b/Libplanet.Action/State/IWorldExtensions.cs index 2919dc35301..994fcd5b8da 100644 --- a/Libplanet.Action/State/IWorldExtensions.cs +++ b/Libplanet.Action/State/IWorldExtensions.cs @@ -113,10 +113,17 @@ public static IWorld TransferAsset( Address sender, Address recipient, FungibleAssetValue value) => - world.SetCurrencyAccount( - world - .GetCurrencyAccount(value.Currency) - .TransferAsset(context, sender, recipient, value)); + context.BlockProtocolVersion > 0 + ? world.SetCurrencyAccount( + world + .GetCurrencyAccount(value.Currency) + .TransferAsset(sender, recipient, value)) +#pragma warning disable CS0618 // Obsolete + : world.SetCurrencyAccount( + world + .GetCurrencyAccount(value.Currency) + .TransferAssetV0(sender, recipient, value)); +#pragma warning restore CS0618 /// /// From f73fafc174d8dbfc4b61c8ed6677b4ed086af996 Mon Sep 17 00:00:00 2001 From: Say Cheong Date: Fri, 10 May 2024 14:12:45 +0900 Subject: [PATCH 2/4] Reorganized code for MintAsset and BurnAsset --- Libplanet.Action/State/CurrencyAccount.cs | 30 +++------------------- Libplanet.Action/State/IWorldExtensions.cs | 30 ++++++++++++++-------- 2 files changed, 24 insertions(+), 36 deletions(-) diff --git a/Libplanet.Action/State/CurrencyAccount.cs b/Libplanet.Action/State/CurrencyAccount.cs index dd43816c2b0..d8e5cca4e49 100644 --- a/Libplanet.Action/State/CurrencyAccount.cs +++ b/Libplanet.Action/State/CurrencyAccount.cs @@ -59,7 +59,6 @@ public FungibleAssetValue GetTotalSupply(Currency currency) } public CurrencyAccount MintAsset( - IActionContext context, Address recipient, FungibleAssetValue value) { @@ -70,22 +69,13 @@ public CurrencyAccount MintAsset( nameof(value), $"The amount to mint, burn, or transfer must be greater than zero: {value}"); } - else if (!Currency.AllowsToMint(context.Signer)) - { - throw new CurrencyPermissionException( - $"Given {nameof(context)}'s signer {context.Signer} does not have " + - $"the authority to mint or burn currency {Currency}.", - context.Signer, - Currency); - } return WorldVersion >= BlockMetadata.CurrencyAccountProtocolVersion - ? MintRawAssetV7(context, recipient, value.RawValue) - : MintRawAssetV0(context, recipient, value.RawValue); + ? MintRawAssetV7(recipient, value.RawValue) + : MintRawAssetV0(recipient, value.RawValue); } public CurrencyAccount BurnAsset( - IActionContext context, Address owner, FungibleAssetValue value) { @@ -96,18 +86,10 @@ public CurrencyAccount BurnAsset( nameof(value), $"The amount to mint, burn, or transfer must be greater than zero: {value}"); } - else if (!Currency.AllowsToMint(context.Signer)) - { - throw new CurrencyPermissionException( - $"Given {nameof(context)}'s signer {context.Signer} does not have " + - $"the authority to mint or burn currency {Currency}.", - context.Signer, - Currency); - } return WorldVersion >= BlockMetadata.CurrencyAccountProtocolVersion - ? BurnRawAssetV7(context, owner, value.RawValue) - : BurnRawAssetV0(context, owner, value.RawValue); + ? BurnRawAssetV7(owner, value.RawValue) + : BurnRawAssetV0(owner, value.RawValue); } public CurrencyAccount TransferAsset( @@ -153,7 +135,6 @@ public IAccount AsAccount() } private CurrencyAccount MintRawAssetV0( - IActionContext context, Address recipient, BigInteger rawValue) { @@ -186,7 +167,6 @@ private CurrencyAccount MintRawAssetV0( } private CurrencyAccount MintRawAssetV7( - IActionContext context, Address recipient, BigInteger rawValue) { @@ -217,7 +197,6 @@ private CurrencyAccount MintRawAssetV7( } private CurrencyAccount BurnRawAssetV0( - IActionContext context, Address owner, BigInteger rawValue) { @@ -249,7 +228,6 @@ private CurrencyAccount BurnRawAssetV0( } private CurrencyAccount BurnRawAssetV7( - IActionContext context, Address owner, BigInteger rawValue) { diff --git a/Libplanet.Action/State/IWorldExtensions.cs b/Libplanet.Action/State/IWorldExtensions.cs index 994fcd5b8da..7afd752252a 100644 --- a/Libplanet.Action/State/IWorldExtensions.cs +++ b/Libplanet.Action/State/IWorldExtensions.cs @@ -36,11 +36,11 @@ public static FungibleAssetValue GetBalance( /// The asset value to mint. /// A new instance that the given is added to 's balance. - /// Thrown when the - /// is less than or equal to 0. /// Thrown when a transaction signer /// (or a miner in case of block actions) is not a member of the 's . + /// Thrown when the + /// is less than or equal to 0. /// Thrown when the sum of the /// to be minted and the current total supply amount of the /// exceeds the @@ -50,9 +50,14 @@ public static IWorld MintAsset( this IWorld world, IActionContext context, Address recipient, - FungibleAssetValue value) => - world.SetCurrencyAccount( - world.GetCurrencyAccount(value.Currency).MintAsset(context, recipient, value)); + FungibleAssetValue value) => value.Currency.AllowsToMint(context.Signer) + ? world.SetCurrencyAccount( + world.GetCurrencyAccount(value.Currency).MintAsset(recipient, value)) + : throw new CurrencyPermissionException( + $"Given {nameof(context)}'s signer {context.Signer} does not have " + + $"the authority to mint or burn currency {value.Currency}.", + context.Signer, + value.Currency); /// /// Burns the fungible asset (i.e., in-game monetary) from @@ -65,11 +70,11 @@ public static IWorld MintAsset( /// The fungible asset to burn. /// A new instance that the given is subtracted from 's balance. - /// Thrown when the - /// is less than or equal to zero. /// Thrown when a transaction signer /// (or a miner in case of block actions) is not a member of the 's . + /// Thrown when the + /// is less than or equal to zero. /// Thrown when the /// has insufficient balance than to burn. [Pure] @@ -77,9 +82,14 @@ public static IWorld BurnAsset( this IWorld world, IActionContext context, Address owner, - FungibleAssetValue value) => - world.SetCurrencyAccount( - world.GetCurrencyAccount(value.Currency).BurnAsset(context, owner, value)); + FungibleAssetValue value) => value.Currency.AllowsToMint(context.Signer) + ? world.SetCurrencyAccount( + world.GetCurrencyAccount(value.Currency).BurnAsset(owner, value)) + : throw new CurrencyPermissionException( + $"Given {nameof(context)}'s signer {context.Signer} does not have " + + $"the authority to mint or burn currency {value.Currency}.", + context.Signer, + value.Currency); /// /// Transfers the fungible asset (i.e., in-game monetary) From e2de5d3167d45baab3a37b04336e11ba3ebf4120 Mon Sep 17 00:00:00 2001 From: Say Cheong Date: Fri, 10 May 2024 17:41:16 +0900 Subject: [PATCH 3/4] Changelog --- CHANGES.md | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 4c6578ad1dc..7f9aae698e2 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -6,9 +6,14 @@ Version 4.5.0 To be released. -Due to changes in [#3780], a network ran with a prior version may not be -compatible with this version, specifically those that ran with -`IAction`s that has used `GetTotalSupply()`. +Due to changes in [#3780] and [#3783], a network ran with a prior version +may not be compatible with this version. Regarding [#3780], a network +that ran with an `IAction` that has used `GetTotalSupply()` with +its execution result dependent on its value may not be compatible. +Regarding [#3783], a network that ran with an `IAction` that has either +used `MintAsset()` and `BurnAsset()` with its execution result dependent on +handling of a possible `Exception` thrown by these methods +may not be compatible. ### Deprecated APIs @@ -27,6 +32,9 @@ compatible with this version, specifically those that ran with - (Libplanet.Action) `IWorldState.GetTotalSupply()` no longer throws a `TotalSupplyNotTrackableException` but returns a zero amount of corresponding `FungibleAssetValue`. [[#3780]] + - (Libplanet.Action) Changed the precednce for the types of `Exception`s + that may be thrown by `IWorldState.MintAsset()` and + `IWorldState.BurnAsset()`. ### Backward-incompatible network protocol changes @@ -53,6 +61,7 @@ compatible with this version, specifically those that ran with [#3778]: https://github.com/planetarium/libplanet/pull/3778 [#3779]: https://github.com/planetarium/libplanet/pull/3779 [#3780]: https://github.com/planetarium/libplanet/pull/3780 +[#3783]: https://github.com/planetarium/libplanet/pull/3783 Version 4.4.2 From d58f50d7869a68968ef2bae590cf39f756318edd Mon Sep 17 00:00:00 2001 From: Say Cheong Date: Mon, 13 May 2024 18:46:22 +0900 Subject: [PATCH 4/4] Cleanup --- CHANGES.md | 4 ++-- Libplanet.Action/State/IStateStoreExtensions.cs | 4 +--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 7f9aae698e2..5f5c35aaf0f 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -33,8 +33,8 @@ may not be compatible. a `TotalSupplyNotTrackableException` but returns a zero amount of corresponding `FungibleAssetValue`. [[#3780]] - (Libplanet.Action) Changed the precednce for the types of `Exception`s - that may be thrown by `IWorldState.MintAsset()` and - `IWorldState.BurnAsset()`. + that may be thrown by `IWorld.MintAsset()` and + `IWorld.BurnAsset()`. ### Backward-incompatible network protocol changes diff --git a/Libplanet.Action/State/IStateStoreExtensions.cs b/Libplanet.Action/State/IStateStoreExtensions.cs index 5b2ff4658f2..c880279cb22 100644 --- a/Libplanet.Action/State/IStateStoreExtensions.cs +++ b/Libplanet.Action/State/IStateStoreExtensions.cs @@ -1,12 +1,10 @@ using System; using System.Collections.Generic; using System.Linq; -using System.Numerics; using System.Security.Cryptography; using System.Text; using Bencodex.Types; using Libplanet.Common; -using Libplanet.Crypto; using Libplanet.Store; using Libplanet.Store.Trie; using Libplanet.Types.Blocks; @@ -157,7 +155,7 @@ internal static IWorld MigrateWorld( } } - // Migrate up to BlockMetadata.ValidatorSetAccountProtocolVersion + // Migrate up to BlockMetadata.CurrencyAccountProtocolVersion // if conditions are met. if (targetVersion >= BlockMetadata.CurrencyAccountProtocolVersion && world.Version < BlockMetadata.CurrencyAccountProtocolVersion)