Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

♻️ Refactor tx gas policy #3567

Merged
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
8 changes: 8 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@ Version 3.9.3

To be released.

Due to changes in [#3567], a network ran with a prior version may not
be compatible with this version, specifically, those that ran with
[Libplanet 2.0.0] and onwards prior to this release that have included
`Transaction`s that aren't compatible with the updated specification in [#3567].

- (Libplanet.Explorer) Added `BlockHashType` and `TxIdType`. [[#3559]]
- (Libplanet.Explorer) Changed `HashDigestSHA256Type` to `HashDigestType<T>`.
[[#3559]]
Expand All @@ -23,12 +28,15 @@ To be released.
- (Libplanet.Explorer) Removed parameters `mysql-server`, `mysql-port`,
`mysql-username`, `mysql-password`, and `mysql-database` from
`Libplanet.Explorer.Executable`. [[#3564]]
- Changed `TxInvoice` to no longer allow negative values for
`MaxGasPrice` and `GasLimit`. [[#3567]]

[#3559]: https://github.com/planetarium/libplanet/pull/3559
[#3560]: https://github.com/planetarium/libplanet/pull/3560
[#3561]: https://github.com/planetarium/libplanet/pull/3561
[#3562]: https://github.com/planetarium/libplanet/pull/3562
[#3564]: https://github.com/planetarium/libplanet/pull/3564
[#3567]: https://github.com/planetarium/libplanet/pull/3567


Version 3.9.2
Expand Down
62 changes: 0 additions & 62 deletions Libplanet.Tests/Action/ActionEvaluatorTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1146,68 +1146,6 @@ public void EvaluateThrowingInsufficientBalanceForGasFee()
evaluations.Single().Exception?.InnerException?.GetType());
}

[Fact]
public void EvaluateMinusGasFee()
{
var privateKey = new PrivateKey();
var address = privateKey.Address;
Currency foo = Currency.Uncapped(
"FOO",
18,
null
);

var freeGasAction = new UseGasAction()
{
GasUsage = 0,
Memo = "FREE",
MintValue = FungibleAssetValue.FromRawValue(foo, 10),
Receiver = address,
};

var payGasAction = new UseGasAction()
{
GasUsage = 1,
Memo = "CHARGE",
};

var store = new MemoryStore();
var stateStore = new TrieStateStore(new MemoryKeyValueStore());
var chain = TestUtils.MakeBlockChain<UseGasAction>(
policy: new BlockPolicy(),
actions: new[]
{
freeGasAction,
},
store: store,
stateStore: stateStore);
var tx = Transaction.Create(
nonce: 0,
privateKey: privateKey,
genesisHash: chain.Genesis.Hash,
maxGasPrice: FungibleAssetValue.FromRawValue(foo, -10),
gasLimit: 5,
actions: new[]
{
payGasAction,
}.ToPlainValues());

chain.StageTransaction(tx);
var miner = new PrivateKey();
Block block = chain.ProposeBlock(miner);

var evaluations = chain.ActionEvaluator.Evaluate(
block, chain.Store.GetStateRootHash(block.PreviousHash));

Assert.False(evaluations[0].InputContext.BlockAction);
Assert.Single(evaluations);
Assert.NotNull(evaluations.Single().Exception);
Assert.NotNull(evaluations.Single().Exception?.InnerException);
Assert.Equal(
typeof(ArgumentOutOfRangeException),
evaluations.Single().Exception?.InnerException?.GetType());
}

[Fact]
public void GenerateRandomSeed()
{
Expand Down
52 changes: 52 additions & 0 deletions Libplanet.Tests/Tx/TxInvoiceTest.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using System.Collections.Immutable;
using Bencodex.Types;
using Libplanet.Action;
using Libplanet.Action.Tests.Common;
using Libplanet.Crypto;
Expand All @@ -18,6 +19,57 @@ public class TxInvoiceTest
public static readonly Address AddressB =
new Address("B61CE2Ce6d28237C1BC6E114616616762f1a12Ab");

[Fact]
public void ConstructorGasConditions()
{
var random = new System.Random();
var genesisHash = random.NextBlockHash();
var timestamp = DateTimeOffset.UtcNow;
var actions = new TxActionList(Array.Empty<IValue>());

_ = new TxInvoice(
genesisHash: genesisHash,
timestamp: timestamp,
actions: actions,
maxGasPrice: null,
gasLimit: null);
_ = new TxInvoice(
genesisHash: genesisHash,
timestamp: timestamp,
actions: actions,
maxGasPrice: Currency.Uncapped("DUMB", 0, null) * 100,
gasLimit: 100);

Assert.Throws<ArgumentException>(() =>
new TxInvoice(
genesisHash: genesisHash,
timestamp: timestamp,
actions: actions,
maxGasPrice: Currency.Uncapped("DUMB", 0, null) * 100,
gasLimit: null));
Assert.Throws<ArgumentException>(() =>
new TxInvoice(
genesisHash: genesisHash,
timestamp: timestamp,
actions: actions,
maxGasPrice: null,
gasLimit: 100));
Assert.Throws<ArgumentException>(() =>
new TxInvoice(
genesisHash: genesisHash,
timestamp: timestamp,
actions: actions,
maxGasPrice: Currency.Uncapped("DUMB", 0, null) * -100,
gasLimit: 100));
Assert.Throws<ArgumentException>(() =>
new TxInvoice(
genesisHash: genesisHash,
timestamp: timestamp,
actions: actions,
maxGasPrice: Currency.Uncapped("DUMB", 0, null) * 100,
gasLimit: -100));
}

[Fact]
public void PlainConstructor()
{
Expand Down
35 changes: 28 additions & 7 deletions Libplanet.Types/Tx/ITxInvoice.cs
Original file line number Diff line number Diff line change
Expand Up @@ -54,18 +54,39 @@ public interface ITxInvoice : IEquatable<ITxInvoice>
TxActionList Actions { get; }

/// <summary>
/// The maximum amount of <see cref="FungibleAssetValue"/> that the transaction author is
/// willing to pay for the transaction.
/// This can be <see langword="null"/> if align the handling of that transaction
/// with the <see cref="IFeeCollector"/>.
/// <para>
/// The maximum amount of <see cref="FungibleAssetValue"/> that the
/// <see cref="Transaction"/>'s author is willing to pay per gas
/// for the <see cref="Transaction"/>.
/// </para>
/// <para>
/// If <see langword="null"/>, gas processing is entirely bypassed.
/// The parity of <see langword="null"/>-ness is always the same as
/// that of <see cref="GasLimit"/>.
/// </para>
/// <para>
/// If not <see langword="null"/>, this value cannot be negative.
/// </para>
/// </summary>
/// <seealso cref="IFeeCollector"/>
/// <seealso cref="GasLimit"/>
FungibleAssetValue? MaxGasPrice { get; }

/// <summary>
/// The limit of the total amount of gas that the transaction will use.
/// This can be <see langword="null"/> if align the handling of that transaction
/// with the <see cref="IFeeCollector"/>.
/// <para>
/// The limit on the total amount of gas that the <see cref="Transaction"/> can use.
/// </para>
/// <para>
/// If <see langword="null"/>, gas processing is entirely bypassed.
/// The parity of <see langword="null"/>-ness is always the same as
/// that of <see cref="MaxGasPrice"/>.
/// </para>
/// <para>
/// If not <see langword="null"/>, this value cannot be negative.
/// </para>
/// </summary>
/// <seealso cref="IFeeCollector"/>
/// <seealso cref="GasLimit"/>
long? GasLimit { get; }
}
}
23 changes: 18 additions & 5 deletions Libplanet.Types/Tx/TxInvoice.cs
Original file line number Diff line number Diff line change
Expand Up @@ -116,12 +116,25 @@ internal TxInvoice(
throw new ArgumentNullException(nameof(updatedAddresses));
}

if (maxGasPrice is null ^ gasLimit is null)
switch (maxGasPrice, gasLimit)
{
throw new ArgumentException(
$"Either {nameof(maxGasPrice)} (null: {maxGasPrice is null}) and " +
$"{nameof(gasLimit)} (null: {gasLimit is null}) must be both null " +
$"or both non-null.");
case (null, null):
break;
case (null, { }):
case ({ }, null):
throw new ArgumentException(
$"Either {nameof(maxGasPrice)} (null: {maxGasPrice is null}) and " +
$"{nameof(gasLimit)} (null: {gasLimit is null}) must be both null " +
$"or both non-null.");
case ({ } mgp, { } gl):
if (mgp.Sign < 0 || gl < 0)
{
throw new ArgumentException(
$"Both {nameof(maxGasPrice)} ({mgp}) and {nameof(gasLimit)} ({gl}) " +
$"must be non-negative.");
}

break;
}

GenesisHash = genesisHash;
Expand Down
Loading