Skip to content

Commit

Permalink
Merge pull request #3783 from greymistcube/refactor/currency-account
Browse files Browse the repository at this point in the history 
♻️ Refactor `CurrencyAccount`
  • Loading branch information
@greymistcube
greymistcube authored May 14, 2024
2 parents 85c21f7 + d58f50d commit c6e6b63
Show file tree
Hide file tree
Showing 4 changed files with 106 additions and 71 deletions.
15 changes: 12 additions & 3 deletions CHANGES.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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 `IWorld.MintAsset()` and
`IWorld.BurnAsset()`.

### Backward-incompatible network protocol changes

Expand All @@ -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
Expand Down
113 changes: 62 additions & 51 deletions Libplanet.Action/State/CurrencyAccount.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ public FungibleAssetValue GetTotalSupply(Currency currency)
}

public CurrencyAccount MintAsset(
IActionContext context,
Address recipient,
FungibleAssetValue value)
{
Expand All @@ -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)
{
Expand All @@ -96,22 +86,13 @@ 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(
IActionContext context,
Address sender,
Address recipient,
FungibleAssetValue value)
Expand All @@ -125,8 +106,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()
Expand All @@ -135,7 +135,6 @@ public IAccount AsAccount()
}

private CurrencyAccount MintRawAssetV0(
IActionContext context,
Address recipient,
BigInteger rawValue)
{
Expand Down Expand Up @@ -168,7 +167,6 @@ private CurrencyAccount MintRawAssetV0(
}

private CurrencyAccount MintRawAssetV7(
IActionContext context,
Address recipient,
BigInteger rawValue)
{
Expand Down Expand Up @@ -199,7 +197,6 @@ private CurrencyAccount MintRawAssetV7(
}

private CurrencyAccount BurnRawAssetV0(
IActionContext context,
Address owner,
BigInteger rawValue)
{
Expand Down Expand Up @@ -231,7 +228,6 @@ private CurrencyAccount BurnRawAssetV0(
}

private CurrencyAccount BurnRawAssetV7(
IActionContext context,
Address owner,
BigInteger rawValue)
{
Expand Down Expand Up @@ -260,7 +256,6 @@ private CurrencyAccount BurnRawAssetV7(
}

private CurrencyAccount TransferRawAssetV7(
IActionContext context,
Address sender,
Address recipient,
BigInteger rawValue)
Expand Down Expand Up @@ -289,8 +284,7 @@ private CurrencyAccount TransferRawAssetV7(
return currencyAccount;
}

private CurrencyAccount TransferRawAssetV0(
IActionContext context,
private CurrencyAccount TransferRawAssetV1(
Address sender,
Address recipient,
BigInteger rawValue)
Expand All @@ -311,29 +305,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;
}

Expand Down
4 changes: 1 addition & 3 deletions Libplanet.Action/State/IStateStoreExtensions.cs
View file
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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)
Expand Down
45 changes: 31 additions & 14 deletions Libplanet.Action/State/IWorldExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,11 @@ public static FungibleAssetValue GetBalance(
/// <param name="value">The asset value to mint.</param>
/// <returns>A new <see cref="IWorld"/> instance that the given <paramref
/// name="value"/> is added to <paramref name="recipient"/>'s balance.</returns>
/// <exception cref="ArgumentOutOfRangeException">Thrown when the <paramref name="value"/>
/// is less than or equal to 0.</exception>
/// <exception cref="CurrencyPermissionException">Thrown when a transaction signer
/// (or a miner in case of block actions) is not a member of the <see
/// cref="FungibleAssetValue.Currency"/>'s <see cref="Currency.Minters"/>.</exception>
/// <exception cref="ArgumentOutOfRangeException">Thrown when the <paramref name="value"/>
/// is less than or equal to 0.</exception>
/// <exception cref="SupplyOverflowException">Thrown when the sum of the
/// <paramref name="value"/> to be minted and the current total supply amount of the
/// <see cref="FungibleAssetValue.Currency"/> exceeds the
Expand All @@ -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);

/// <summary>
/// Burns the fungible asset <paramref name="value"/> (i.e., in-game monetary) from
Expand All @@ -65,21 +70,26 @@ public static IWorld MintAsset(
/// <param name="value">The fungible asset <paramref name="value"/> to burn.</param>
/// <returns>A new <see cref="IWorld"/> instance that the given <paramref
/// name="value"/> is subtracted from <paramref name="owner"/>'s balance.</returns>
/// <exception cref="ArgumentOutOfRangeException">Thrown when the <paramref name="value"/>
/// is less than or equal to zero.</exception>
/// <exception cref="CurrencyPermissionException">Thrown when a transaction signer
/// (or a miner in case of block actions) is not a member of the <see
/// cref="FungibleAssetValue.Currency"/>'s <see cref="Currency.Minters"/>.</exception>
/// <exception cref="ArgumentOutOfRangeException">Thrown when the <paramref name="value"/>
/// is less than or equal to zero.</exception>
/// <exception cref="InsufficientBalanceException">Thrown when the <paramref name="owner"/>
/// has insufficient balance than <paramref name="value"/> to burn.</exception>
[Pure]
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);

/// <summary>
/// Transfers the fungible asset <paramref name="value"/> (i.e., in-game monetary)
Expand Down Expand Up @@ -113,10 +123,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

/// <summary>
/// <para>
Expand Down

0 comments on commit c6e6b63

Please sign in to comment.