Skip to content

Commit 0fa7bfc

Browse files
quantumagiquantumagi
authored
Refactor WhitelistedHashesRepository (#1090)
* Refactor WhitelistedHashesRepository * Add missing bits * Fix constructor * Fix * Enforce poll application in height and id order * Make UnProcessBlock deterministic
1 parent b3e5ba5 commit 0fa7bfc

10 files changed

Lines changed: 148 additions & 72 deletions

File tree

‎src/Stratis.Bitcoin.Features.PoA.Tests/VotingManagerTests.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,8 @@ public VotingManagerTests()
2424
this.changesApplied = new List<VotingData>();
2525
this.changesReverted = new List<VotingData>();
2626

27-
this.resultExecutorMock.Setup(x => x.ApplyChange(It.IsAny<VotingData>())).Callback((VotingData data) => this.changesApplied.Add(data));
28-
this.resultExecutorMock.Setup(x => x.RevertChange(It.IsAny<VotingData>())).Callback((VotingData data) => this.changesReverted.Add(data));
27+
this.resultExecutorMock.Setup(x => x.ApplyChange(It.IsAny<VotingData>(), It.IsAny<int>())).Callback((VotingData data, int _) => this.changesApplied.Add(data));
28+
this.resultExecutorMock.Setup(x => x.RevertChange(It.IsAny<VotingData>(), It.IsAny<int>())).Callback((VotingData data, int _) => this.changesReverted.Add(data));
2929
}
3030

3131
[Fact]

‎src/Stratis.Bitcoin.Features.PoA/PoAFeature.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ public override Task InitializeAsync()
137137
}
138138

139139
this.federationManager.Initialize();
140-
this.whitelistedHashesRepository.Initialize();
140+
this.whitelistedHashesRepository.Initialize(this.votingManager);
141141

142142
if (!this.votingManager.Synchronize(this.chainIndexer.Tip))
143143
throw new System.OperationCanceledException();

‎src/Stratis.Bitcoin.Features.PoA/Voting/PollResultExecutor.cs‎

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,13 @@ public interface IPollResultExecutor
88
{
99
/// <summary>Applies effect of <see cref="VotingData"/>.</summary>
1010
/// <param name="data">See <see cref="VotingData"/>.</param>
11-
void ApplyChange(VotingData data);
11+
/// <param name="executionHeight">The height from which the change should take effect.</param>
12+
void ApplyChange(VotingData data, int executionHeight);
1213

1314
/// <summary>Reverts effect of <see cref="VotingData"/>.</summary>
1415
/// <param name="data">See <see cref="VotingData"/>.</param>
15-
void RevertChange(VotingData data);
16+
/// <param name="executionHeight">The height from which the change should take effect.</param>
17+
void RevertChange(VotingData data, int executionHeight);
1618

1719
/// <summary>Converts <see cref="VotingData"/> to a human readable format.</summary>
1820
/// <param name="data">See <see cref="VotingData"/>.</param>
@@ -40,7 +42,7 @@ public PollResultExecutor(IFederationManager federationManager, ILoggerFactory l
4042
}
4143

4244
/// <inheritdoc />
43-
public void ApplyChange(VotingData data)
45+
public void ApplyChange(VotingData data, int executionHeight)
4446
{
4547
switch (data.Key)
4648
{
@@ -53,17 +55,17 @@ public void ApplyChange(VotingData data)
5355
break;
5456

5557
case VoteKey.WhitelistHash:
56-
this.AddHash(data.Data);
58+
this.AddHash(data.Data, executionHeight);
5759
break;
5860

5961
case VoteKey.RemoveHash:
60-
this.RemoveHash(data.Data);
62+
this.RemoveHash(data.Data, executionHeight);
6163
break;
6264
}
6365
}
6466

6567
/// <inheritdoc />
66-
public void RevertChange(VotingData data)
68+
public void RevertChange(VotingData data, int executionHeight)
6769
{
6870
switch (data.Key)
6971
{
@@ -76,11 +78,11 @@ public void RevertChange(VotingData data)
7678
break;
7779

7880
case VoteKey.WhitelistHash:
79-
this.RemoveHash(data.Data);
81+
this.RemoveHash(data.Data, executionHeight);
8082
break;
8183

8284
case VoteKey.RemoveHash:
83-
this.AddHash(data.Data);
85+
this.AddHash(data.Data, executionHeight);
8486
break;
8587
}
8688
}
@@ -118,32 +120,32 @@ public void RemoveFederationMember(byte[] federationMemberBytes)
118120
this.federationManager.RemoveFederationMember(federationMember);
119121
}
120122

121-
private void AddHash(byte[] hashBytes)
123+
private void AddHash(byte[] hashBytes, int executionHeight)
122124
{
123125
try
124126
{
125127
var hash = new uint256(hashBytes);
126128

127-
this.whitelistedHashesRepository.AddHash(hash);
129+
this.whitelistedHashesRepository.AddHash(hash, executionHeight);
128130
}
129131
catch (FormatException e)
130132
{
131133
this.logger.LogWarning("Hash had incorrect format: '{0}'.", e.ToString());
132134
}
133135
}
134136

135-
private void RemoveHash(byte[] hashBytes)
137+
private void RemoveHash(byte[] hashBytes, int executionHeight)
136138
{
137139
try
138140
{
139141
var hash = new uint256(hashBytes);
140142

141-
this.whitelistedHashesRepository.RemoveHash(hash);
143+
this.whitelistedHashesRepository.RemoveHash(hash, executionHeight);
142144
}
143145
catch (FormatException e)
144146
{
145147
this.logger.LogWarning("Hash had incorrect format: '{0}'.", e.ToString());
146148
}
147149
}
148150
}
149-
}
151+
}

‎src/Stratis.Bitcoin.Features.PoA/Voting/VotingManager.cs‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -531,9 +531,7 @@ private void ProcessBlock(PollsRepository.Transaction transaction, ChainedHeader
531531
{
532532
this.signals.Publish(new VotingManagerProcessBlock(chBlock, transaction));
533533

534-
bool pollsRepositoryModified = false;
535-
536-
foreach (Poll poll in this.polls.GetPollsToExecuteOrExpire(chBlock.ChainedHeader.Height))
534+
foreach (Poll poll in this.polls.GetPollsToExecuteOrExpire(chBlock.ChainedHeader.Height).OrderBy(p => p.Id))
537535
{
538536
if (!poll.IsApproved)
539537
{
@@ -547,7 +545,7 @@ private void ProcessBlock(PollsRepository.Transaction transaction, ChainedHeader
547545
else
548546
{
549547
this.logger.LogDebug("Applying poll '{0}'.", poll);
550-
this.pollResultExecutor.ApplyChange(poll.VotingData);
548+
this.pollResultExecutor.ApplyChange(poll.VotingData, chBlock.ChainedHeader.Height);
551549

552550
this.polls.AdjustPoll(poll, poll => poll.PollExecutedBlockData = new HashHeightPair(chBlock.ChainedHeader));
553551
transaction.UpdatePoll(poll);
@@ -691,16 +689,22 @@ private void UnProcessBlock(PollsRepository.Transaction transaction, ChainedHead
691689
{
692690
lock (this.locker)
693691
{
694-
foreach (Poll poll in this.polls.Where(x => !x.IsPending && x.PollExecutedBlockData?.Hash == chBlock.ChainedHeader.HashBlock).ToList())
692+
foreach (Poll poll in this.polls
693+
.Where(x => !x.IsPending && x.PollExecutedBlockData?.Hash == chBlock.ChainedHeader.HashBlock)
694+
.OrderByDescending(p => p.Id)
695+
.ToList())
695696
{
696697
this.logger.LogDebug("Reverting poll execution '{0}'.", poll);
697-
this.pollResultExecutor.RevertChange(poll.VotingData);
698+
this.pollResultExecutor.RevertChange(poll.VotingData, chBlock.ChainedHeader.Height);
698699

699700
this.polls.AdjustPoll(poll, poll => poll.PollExecutedBlockData = null);
700701
transaction.UpdatePoll(poll);
701702
}
702703

703-
foreach (Poll poll in this.polls.Where(x => x.IsExpired && !PollsRepository.IsPollExpiredAt(x, chBlock.ChainedHeader.Height - 1, this.network as PoANetwork)).ToList())
704+
foreach (Poll poll in this.polls
705+
.Where(x => x.IsExpired && !PollsRepository.IsPollExpiredAt(x, chBlock.ChainedHeader.Height - 1, this.network as PoANetwork))
706+
.OrderByDescending(p => p.Id)
707+
.ToList())
704708
{
705709
this.logger.LogDebug("Reverting poll expiry '{0}'.", poll);
706710

‎src/Stratis.Bitcoin.Features.PoA/Voting/WhitelistedHashesRepository.cs‎

Lines changed: 101 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,93 +1,163 @@
1-
using System.Collections.Generic;
1+
using System;
2+
using System.Collections.Generic;
3+
using System.Linq;
24
using Microsoft.Extensions.Logging;
35
using NBitcoin;
4-
using Stratis.Bitcoin.Persistence;
6+
using Stratis.Bitcoin.Utilities;
57

68
namespace Stratis.Bitcoin.Features.PoA.Voting
79
{
810
public class WhitelistedHashesRepository : IWhitelistedHashesRepository
911
{
10-
private const string dbKey = "hashesList";
11-
12-
private readonly IKeyValueRepository kvRepository;
13-
1412
/// <summary>Protects access to <see cref="whitelistedHashes"/>.</summary>
1513
private readonly object locker;
1614

1715
private readonly ILogger logger;
1816

19-
private List<uint256> whitelistedHashes;
17+
private readonly PoAConsensusOptions poaConsensusOptions;
2018

21-
public WhitelistedHashesRepository(ILoggerFactory loggerFactory, IKeyValueRepository kvRepository)
19+
// Dictionary of hash histories. Even list entries are additions and odd entries are removals.
20+
private Dictionary<uint256, int[]> whitelistedHashes;
21+
22+
public WhitelistedHashesRepository(ILoggerFactory loggerFactory, Network network)
2223
{
23-
this.kvRepository = kvRepository;
2424
this.locker = new object();
2525

2626
this.logger = loggerFactory.CreateLogger(this.GetType().FullName);
27+
this.poaConsensusOptions = network.Consensus.Options as PoAConsensusOptions;
28+
}
2729

28-
// Load this before initialize to ensure its available to when the Mempool feature initializes.
29-
lock (this.locker)
30+
public class PollComparer : IComparer<(int height, int id)>
31+
{
32+
public int Compare((int height, int id) poll1, (int height, int id) poll2)
3033
{
31-
this.whitelistedHashes = this.kvRepository.LoadValueJson<List<uint256>>(dbKey) ?? new List<uint256>();
34+
int cmp = poll1.height.CompareTo(poll2.height);
35+
if (cmp != 0)
36+
return cmp;
37+
38+
return poll1.id.CompareTo(poll2.id);
3239
}
3340
}
3441

35-
public void Initialize()
42+
static PollComparer pollComparer = new PollComparer();
43+
44+
private void GetWhitelistedHashesFromExecutedPolls(VotingManager votingManager)
3645
{
46+
lock (this.locker)
47+
{
48+
var federation = new List<IFederationMember>(this.poaConsensusOptions.GenesisFederationMembers);
49+
50+
IEnumerable<Poll> executedPolls = votingManager.GetExecutedPolls().WhitelistPolls();
51+
foreach (Poll poll in executedPolls.OrderBy(a => (a.PollExecutedBlockData.Height, a.Id), pollComparer))
52+
{
53+
var hash = new uint256(poll.VotingData.Data);
54+
55+
if (poll.VotingData.Key == VoteKey.WhitelistHash)
56+
{
57+
this.AddHash(hash, poll.PollExecutedBlockData.Height);
58+
}
59+
else if (poll.VotingData.Key == VoteKey.RemoveHash)
60+
{
61+
this.RemoveHash(hash, poll.PollExecutedBlockData.Height);
62+
}
63+
}
64+
}
3765
}
3866

39-
private void SaveHashes()
67+
public void Initialize(VotingManager votingManager)
4068
{
69+
// TODO: Must call Initialize before the Mempool rules try to use this class.
4170
lock (this.locker)
4271
{
43-
this.kvRepository.SaveValueJson(dbKey, this.whitelistedHashes);
72+
this.whitelistedHashes = new Dictionary<uint256, int[]>();
73+
this.GetWhitelistedHashesFromExecutedPolls(votingManager);
4474
}
4575
}
4676

47-
public void AddHash(uint256 hash)
77+
public void AddHash(uint256 hash, int executionHeight)
4878
{
4979
lock (this.locker)
5080
{
51-
if (this.whitelistedHashes.Contains(hash))
81+
// Retrieve the whitelist history for this hash.
82+
if (!this.whitelistedHashes.TryGetValue(hash, out int[] history))
5283
{
53-
this.logger.LogTrace("(-)[ALREADY_EXISTS]");
84+
this.whitelistedHashes[hash] = new int[] { executionHeight };
5485
return;
5586
}
5687

57-
this.whitelistedHashes.Add(hash);
88+
// Keep all history up to and including the executionHeight.
89+
int keep = BinarySearch.BinaryFindFirst((k) => k == history.Length || history[k] > executionHeight, 0, history.Length + 1);
90+
Array.Resize(ref history, keep | 1);
91+
this.whitelistedHashes[hash] = history;
92+
93+
// If the history is an even length then add the addition height to signify addition.
94+
if ((keep % 2) == 0)
95+
{
96+
// Add an even indexed entry to signify an addition.
97+
history[keep] = executionHeight;
98+
return;
99+
}
100+
101+
this.logger.LogTrace("(-)[HASH_ALREADY_EXISTS]");
102+
return;
103+
}
104+
}
105+
106+
public void RemoveHash(uint256 hash, int executionHeight)
107+
{
108+
lock (this.locker)
109+
{
110+
// Retrieve the whitelist history for this hash.
111+
if (this.whitelistedHashes.TryGetValue(hash, out int[] history))
112+
{
113+
// Keep all history up to and including the executionHeight.
114+
int keep = BinarySearch.BinaryFindFirst((k) => k == history.Length || history[k] >= executionHeight, 0, history.Length + 1);
115+
Array.Resize(ref history, (keep + 1) & ~1);
116+
this.whitelistedHashes[hash] = history;
117+
118+
// If the history is an odd length then add the removal height to signify removal.
119+
if ((keep % 2) != 0)
120+
{
121+
history[keep] = executionHeight;
122+
return;
123+
}
124+
}
58125

59-
this.SaveHashes();
126+
this.logger.LogTrace("(-)[HASH_DOESNT_EXIST]");
127+
return;
60128
}
61129
}
62130

63-
public void RemoveHash(uint256 hash)
131+
private bool ExistsHash(uint256 hash, int blockHeight)
64132
{
65133
lock (this.locker)
66134
{
67-
bool removed = this.whitelistedHashes.Remove(hash);
135+
// Retrieve the whitelist history for this hash.
136+
if (!this.whitelistedHashes.TryGetValue(hash, out int[] history))
137+
return false;
68138

69-
if (removed)
70-
this.SaveHashes();
139+
int keep = BinarySearch.BinaryFindFirst((k) => k == history.Length || history[k] > blockHeight, 0, history.Length + 1);
140+
return (keep % 2) != 0;
71141
}
72142
}
73143

74-
public List<uint256> GetHashes()
144+
public List<uint256> GetHashes(int blockHeight = int.MaxValue)
75145
{
76146
lock (this.locker)
77147
{
78-
return new List<uint256>(this.whitelistedHashes);
148+
return this.whitelistedHashes.Where(k => ExistsHash(k.Key, blockHeight)).Select(k => k.Key).ToList();
79149
}
80150
}
81151
}
82152

83153
public interface IWhitelistedHashesRepository
84154
{
85-
void AddHash(uint256 hash);
155+
void AddHash(uint256 hash, int executionHeight);
86156

87-
void RemoveHash(uint256 hash);
157+
void RemoveHash(uint256 hash, int executionHeight);
88158

89-
List<uint256> GetHashes();
159+
List<uint256> GetHashes(int blockHeight = int.MaxValue);
90160

91-
void Initialize();
161+
void Initialize(VotingManager votingManager);
92162
}
93-
}
163+
}

0 commit comments

Comments
 (0)