Skip to content

Commit

Permalink
Simplify estimation code (#12571)
Browse files Browse the repository at this point in the history
* Simplify estimation code

* Do not assume the target!
  • Loading branch information
lontivero committed Mar 1, 2024
1 parent a571aa5 commit 0a439f2
Show file tree
Hide file tree
Showing 9 changed files with 32 additions and 95 deletions.
4 changes: 1 addition & 3 deletions WalletWasabi.Fluent/Helpers/TransactionFeeHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ namespace WalletWasabi.Fluent.Helpers;
public static class TransactionFeeHelper
{
private static readonly AllFeeEstimate TestNetFeeEstimates = new(
EstimateSmartFeeMode.Conservative,
new Dictionary<int, int>
{
[1] = 17,
Expand All @@ -28,8 +27,7 @@ public static class TransactionFeeHelper
[144] = 2,
[432] = 1,
[1008] = 1
},
false);
});

public static async Task<AllFeeEstimate> GetFeeEstimatesWhenReadyAsync(Wallet wallet, CancellationToken cancellationToken)
{
Expand Down
20 changes: 9 additions & 11 deletions WalletWasabi.Tests/UnitTests/AllFeeEstimateTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,14 @@ public void Serialization()
{ 3, 20 },
{ 19, 1 }
};
var allFee = new AllFeeEstimate(EstimateSmartFeeMode.Conservative, estimations, true);
var allFee = new AllFeeEstimate(estimations);
var serialized = JsonConvert.SerializeObject(allFee);
var deserialized = JsonConvert.DeserializeObject<AllFeeEstimate>(serialized);

Assert.NotNull(deserialized);
Assert.Equal(estimations[2], deserialized!.Estimations[2]);
Assert.Equal(estimations[3], deserialized!.Estimations[3]);
Assert.Equal(estimations[19], deserialized!.Estimations[36]);
Assert.Equal(EstimateSmartFeeMode.Conservative, deserialized!.Type);
}

[Fact]
Expand All @@ -49,7 +48,7 @@ public void OrdersByTarget()
{ 20, 1 }
};

var allFee = new AllFeeEstimate(EstimateSmartFeeMode.Conservative, estimations, true);
var allFee = new AllFeeEstimate(estimations);
Assert.Equal(estimations[2], allFee.Estimations[2]);
Assert.Equal(estimations[3], allFee.Estimations[3]);
Assert.Equal(estimations[19], allFee.Estimations[36]);
Expand All @@ -64,7 +63,7 @@ public void HandlesDuplicate()
{ 3, 20 }
};

var allFee = new AllFeeEstimate(EstimateSmartFeeMode.Conservative, estimations, true);
var allFee = new AllFeeEstimate(estimations);
Assert.Single(allFee.Estimations);
Assert.Equal(estimations[2], allFee.Estimations[2]);
}
Expand All @@ -78,7 +77,7 @@ public void HandlesOne()
{ 1, 20 }
};

var allFees = new AllFeeEstimate(EstimateSmartFeeMode.Conservative, estimations, true);
var allFees = new AllFeeEstimate(estimations);
Assert.Single(allFees.Estimations);
Assert.Equal(estimations[1], allFees.Estimations[2]);

Expand All @@ -89,7 +88,7 @@ public void HandlesOne()
{ 2, 21 }
};

allFees = new AllFeeEstimate(EstimateSmartFeeMode.Conservative, estimations, true);
allFees = new AllFeeEstimate(estimations);
Assert.Single(allFees.Estimations);
Assert.Equal(estimations[2], allFees.Estimations[2]);
}
Expand All @@ -102,7 +101,7 @@ public void EndOfTheRange()
{ 1007, 20 }
};

var allFees = new AllFeeEstimate(EstimateSmartFeeMode.Conservative, estimations, true);
var allFees = new AllFeeEstimate(estimations);
var est = Assert.Single(allFees.Estimations);
Assert.Equal(1008, est.Key);
}
Expand All @@ -116,7 +115,7 @@ public void HandlesInconsistentData()
{ 3, 21 }
};

var allFee = new AllFeeEstimate(EstimateSmartFeeMode.Conservative, estimations, true);
var allFee = new AllFeeEstimate(estimations);
Assert.Single(allFee.Estimations);
Assert.Equal(estimations[2], allFee.Estimations[2]);

Expand All @@ -129,7 +128,7 @@ public void HandlesInconsistentData()
{ 6, 4 },
};

allFee = new AllFeeEstimate(EstimateSmartFeeMode.Conservative, estimations, true);
allFee = new AllFeeEstimate(estimations);
Assert.Equal(2, allFee.Estimations.Count);
Assert.Equal(estimations[2], allFee.Estimations[2]);
Assert.Equal(estimations[6], allFee.Estimations[6]);
Expand Down Expand Up @@ -226,7 +225,6 @@ target switch
};

var allFee = await mockRpc.EstimateAllFeeAsync();
Assert.True(allFee.IsAccurate);
Assert.Equal(3, allFee.Estimations.Count);
Assert.Equal(99, allFee.Estimations[2]);
Assert.Equal(75, allFee.Estimations[6]);
Expand Down Expand Up @@ -375,7 +373,7 @@ public void WildEstimations()
{ 18, 1 } // 3h
};

var allFee = new AllFeeEstimate(EstimateSmartFeeMode.Conservative, estimations, true);
var allFee = new AllFeeEstimate(estimations);

Assert.Equal(7, allFee.WildEstimations.Count());
Assert.Equal(new FeeRate(102m), allFee.WildEstimations.First().feeRate); // 20m
Expand Down
1 change: 0 additions & 1 deletion WalletWasabi.Tests/UnitTests/BitcoinCore/RpcBasedTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,6 @@ public async Task AllFeeEstimateAsync()
Assert.Equal(7, estimations.Estimations.Count);
Assert.True(estimations.Estimations.First().Key < estimations.Estimations.Last().Key);
Assert.True(estimations.Estimations.First().Value > estimations.Estimations.Last().Value);
Assert.Equal(EstimateSmartFeeMode.Conservative, estimations.Type);
}
finally
{
Expand Down
6 changes: 1 addition & 5 deletions WalletWasabi/BitcoinCore/Monitoring/RpcFeeProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,8 @@ protected override async Task ActionAsync(CancellationToken cancel)
{
var allFeeEstimate = await RpcClient.EstimateAllFeeAsync(cancel).ConfigureAwait(false);

// If Core was running for a day already && it's synchronized, then we can be pretty sure that the estimate is accurate.
// It could also be accurate if Core was only shut down for a few minutes, but that's hard to figure out.
allFeeEstimate.IsAccurate = RpcMonitor.RpcStatus.Synchronized && await RpcClient.UptimeAsync(cancel).ConfigureAwait(false) > TimeSpan.FromDays(1);

LastAllFeeEstimate = allFeeEstimate;
if (allFeeEstimate?.Estimations?.Any() is true)
if (allFeeEstimate.Estimations.Any())
{
AllFeeEstimateArrived?.Invoke(this, allFeeEstimate);
}
Expand Down
31 changes: 4 additions & 27 deletions WalletWasabi/Blockchain/Analysis/FeesEstimation/AllFeeEstimate.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,10 @@ namespace WalletWasabi.Blockchain.Analysis.FeesEstimation;
public class AllFeeEstimate : IEquatable<AllFeeEstimate>
{
[JsonConstructor]
public AllFeeEstimate(EstimateSmartFeeMode type, IDictionary<int, int> estimations, bool isAccurate)
public AllFeeEstimate(IDictionary<int, int> estimations)
{
Guard.NotNullOrEmpty(nameof(estimations), estimations);

Type = type;
IsAccurate = isAccurate;

var targets = Constants.ConfirmationTargets.Prepend(1).ToArray();
var targetRanges = targets.Skip(1).Zip(targets.Skip(1).Prepend(0), (x, y) => (Start: y, End: x));

Expand All @@ -47,20 +44,11 @@ public AllFeeEstimate(EstimateSmartFeeMode type, IDictionary<int, int> estimatio
}
}

public AllFeeEstimate(AllFeeEstimate other, bool isAccurate)
: this(other.Type, other.Estimations, isAccurate)
public AllFeeEstimate(AllFeeEstimate other)
: this(other.Estimations)
{
}

[JsonProperty]
public EstimateSmartFeeMode Type { get; }

/// <summary>
/// Gets a value indicating whether the fee has been fetched from a fully synced node.
/// </summary>
[JsonProperty]
public bool IsAccurate { get; set; }

/// <summary>
/// Gets the fee estimations: int: fee target, int: satoshi/vByte
/// </summary>
Expand Down Expand Up @@ -206,12 +194,11 @@ public TimeSpan EstimateConfirmationTime(FeeRate feeRate)

public override int GetHashCode()
{
int hash = Type.GetHashCode();
int hash = 13;
foreach (KeyValuePair<int, int> est in Estimations)
{
hash ^= est.Key.GetHashCode() ^ est.Value.GetHashCode();
}
hash ^= IsAccurate.GetHashCode();

return hash;
}
Expand All @@ -228,16 +215,6 @@ public override int GetHashCode()
return false;
}

if (x.Type != y.Type)
{
return false;
}

if (x.IsAccurate != y.IsAccurate)
{
return false;
}

bool equal = false;
if (x.Estimations.Count == y.Estimations.Count) // Require equal count.
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System.Collections.Generic;
using Microsoft.Extensions.Hosting;
using System.Linq;
using System.Threading;
Expand Down Expand Up @@ -53,12 +54,12 @@ public async Task StopAsync(CancellationToken cancellationToken)
await ProcessingEvents.WhenAllAsync().ConfigureAwait(false);
}

private void OnAllFeeEstimateArrived(object? sender, AllFeeEstimate? fees)
private void OnAllFeeEstimateArrived(object? sender, AllFeeEstimate fees)
{
using (RunningTasks.RememberWith(ProcessingEvents))
{
// Only go further if we have estimations.
if (fees?.Estimations?.Any() is not true)
if (fees.Estimations.Any() is not true)
{
return;
}
Expand All @@ -81,55 +82,28 @@ private void OnAllFeeEstimateArrived(object? sender, AllFeeEstimate? fees)
}
else
{
if (rpcProvider.LastAllFeeEstimate?.IsAccurate is true && !rpcProvider.InError)
if (!rpcProvider.InError)
{
// If user's full node is properly serving data, then we don't care about the third party.
return;
}
else
{
if (fees.IsAccurate)
{
// If the third party is properly serving accurate data then, this is the best we got.
notify = SetAllFeeEstimate(fees);
}
else
{
// If neither user's full node, nor third party is ready, then let's try our best effort figuring out which data looks better:
notify = SetAllFeeEstimateIfLooksBetter(fees);
}
}

// If the third party is properly serving accurate data then, this is the best we got.
notify = SetAllFeeEstimate(fees);
}
}
else if (sender is RpcFeeProvider rpcProvider)
{
if (fees.IsAccurate)
{
// If user's full node is properly serving data, we're done here.
notify = SetAllFeeEstimate(fees);
}
else
{
if (ThirdPartyFeeProvider.InError)
{
// If neither user's full node, nor the third party is ready, then let's try our best effort figuring out which data looks better:
notify = SetAllFeeEstimateIfLooksBetter(fees);
}
else
{
// If the user's full node isn't ready, but the third party is, then let's leave it to the third party.
return;
}
}
// If user's full node is properly serving data, we're done here.
notify = SetAllFeeEstimate(fees);
}
}

if (notify)
{
var accuracy = fees.IsAccurate ? "Accurate" : "Inaccurate";
var from = fees.Estimations.First();
var to = fees.Estimations.Last();
Logger.LogInfo($"{accuracy} fee rates are acquired from {sender?.GetType()?.Name} ranging from target {from.Key} blocks at {from.Value} sat/vByte to target {to.Key} blocks at {to.Value} sat/vByte.");
Logger.LogInfo($"Fee rates are acquired from {sender?.GetType()?.Name} ranging from target {from.Key} blocks at {from.Value} sat/vByte to target {to.Key} blocks at {to.Value} sat/vByte.");
AllFeeEstimateChanged?.Invoke(this, fees);
}
}
Expand All @@ -141,7 +115,7 @@ private bool SetAllFeeEstimateIfLooksBetter(AllFeeEstimate? fees)
var current = AllFeeEstimate;
if (fees is null
|| fees == current
|| (current is not null && ((!fees.IsAccurate && current.IsAccurate) || fees.Estimations.Count <= current.Estimations.Count)))
|| (current is not null && fees.Estimations.Count <= current.Estimations.Count))
{
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ private bool SetAllFeeEstimateIfLooksBetter(AllFeeEstimate? fees)
var current = LastAllFeeEstimate;
if (fees is null
|| fees == current
|| (current is not null && ((!fees.IsAccurate && current.IsAccurate) || fees.Estimations.Count <= current.Estimations.Count)))
|| (current is not null && fees.Estimations.Count <= current.Estimations.Count))
{
return false;
}
Expand Down
13 changes: 4 additions & 9 deletions WalletWasabi/Extensions/RPCClientExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -74,12 +74,7 @@ public static async Task<AllFeeEstimate> EstimateAllFeeAsync(this IRPCClient rpc
? smartEstimations
: SmartEstimationsWithMempoolInfo(smartEstimations, mempoolInfo);

var rpcStatus = await rpc.GetRpcStatusAsync(cancel).ConfigureAwait(false);

return new AllFeeEstimate(
EstimateMode,
finalEstimations,
rpcStatus.Synchronized);
return new AllFeeEstimate(finalEstimations);
}

private static FeeRateByConfirmationTarget SmartEstimationsWithMempoolInfo(FeeRateByConfirmationTarget smartEstimations, MemPoolInfo mempoolInfo)
Expand Down Expand Up @@ -139,9 +134,9 @@ private static async Task<FeeRateByConfirmationTarget> GetFeeEstimationsAsync(IR

// EstimateSmartFeeAsync returns the block number where estimate was found - not always what the requested one.
return rpcFeeEstimationTasks
.Zip(Constants.ConfirmationTargets, (task, target) => (task, target))
.Where(x => x.task.IsCompletedSuccessfully)
.Select(x => (x.target, feeRate: x.task.Result.FeeRate))
.Where(x => x.IsCompletedSuccessfully)
.Select(x => (target: x.Result.Blocks, feeRate: x.Result.FeeRate))
.DistinctBy(x => x.target)
.ToDictionary(x => x.target, x => (int)Math.Ceiling(x.feeRate.SatoshiPerByte));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,6 @@ public async Task<AllFeeEstimate> GetFeeEstimatesAsync(CancellationToken cancel)
myDic.Add(int.Parse(elem.Name), (int)Math.Ceiling(elem.Value.GetDouble()));
}

return new AllFeeEstimate(EstimateSmartFeeMode.Conservative, myDic, isAccurate: true);
return new AllFeeEstimate(myDic);
}
}

0 comments on commit 0a439f2

Please sign in to comment.