[refactoring] FeeStrategy implementation
#11651
Conversation
kiminuo
force-pushed
the
feature/2023-10-08-FeeStrategy-refactoring
branch
from
8c4da97
to
0418f89
Compare
| } | ||
|
|
||
| public static FeeStrategy TwentyMinutesConfirmationTargetStrategy { get; } = CreateFromConfirmationTarget(Constants.TwentyMinutesConfirmationTarget); | ||
| public static FeeStrategy OneDayConfirmationTargetStrategy { get; } = CreateFromConfirmationTarget(Constants.OneDayConfirmationTarget); | ||
| public static FeeStrategy SevenDaysConfirmationTargetStrategy { get; } = CreateFromConfirmationTarget(Constants.SevenDaysConfirmationTarget); | ||
|
|
||
| public static FeeStrategy CreateFromConfirmationTarget(int confirmationTarget) => new(FeeStrategyType.Target, confirmationTarget: confirmationTarget, feeRate: null); | ||
| public static FeeStrategy CreateFromConfirmationTarget(int confirmationTarget) | ||
| => new(confirmationTarget: confirmationTarget); |
There was a problem hiding this comment.
Just to easily compare with L67.
| using WalletWasabi.Helpers; | ||
|
|
||
| namespace WalletWasabi.Blockchain.TransactionBuilding; | ||
|
|
||
| public class FeeStrategy | ||
| { | ||
| public static readonly FeeRate MinimumFeeRate = new(1m); |
There was a problem hiding this comment.
Avoids allocations.
There was a problem hiding this comment.
Nit: Only one reference to MinimumFeeRate. Maybe it could be private?
There was a problem hiding this comment.
The idea is that one can compare to that number if FeeRate strategy is used. You are right that it's not used at the moment. It's a constant in a way so I think it's OK to be public.
If you feel strongly against it, please change it in a follow-up PR.
| private int? _target; | ||
| private FeeRate? _rate; | ||
|
|
||
| private FeeStrategy(FeeStrategyType type, int? confirmationTarget, FeeRate? feeRate) | ||
| private FeeStrategy(int confirmationTarget) |
There was a problem hiding this comment.
There are two FeeStrategyTypes, so 2 ctors is a simple approach to model that.
| @@ -1,4 +1,4 @@ | |||
| using System.Collections.Generic; | |||
| using System.Collections.Generic; | |||
kiminuo
force-pushed
the
feature/2023-10-08-FeeStrategy-refactoring
branch
from
2a1a280
to
0796e60
Compare
| using WalletWasabi.Helpers; | ||
|
|
||
| namespace WalletWasabi.Blockchain.TransactionBuilding; | ||
|
|
||
| public class FeeStrategy | ||
| { | ||
| public static readonly FeeRate MinimumFeeRate = new(1m); |
There was a problem hiding this comment.
Nit: Only one reference to MinimumFeeRate. Maybe it could be private?
|
Ha, you asked @Szpoti for review. He can certainly review the PR and I can address potential feedback in a follow-up PR. |
Simplify
FeeStrategyimplementation and remove 2 warnings in the process.Easy to review commit by commit.