fix(xdc): persist epoch rewards on canonical block added (#12288)

* fix no persist rewards on block processing path

* log on exception

* use TransactionProcessor to read when doing rewards

* remove pruning and make rewards lookups hash based

* unnecessary using

* simplify cache rewards instead

* save epoch in foreground

* updated tests

* remove using

* Set reward cache after validation

* test changes

* add missing fields in config

* reduce logs a bit

* added missing LimitPenaltyEpoch

* format

Author: ak88

src/Nethermind/Nethermind.Consensus/Processing/BlockProcessor.cs

@@ -100,13 +100,18 @@ private void ValidateProcessedBlock(Block suggestedBlock, ProcessingOptions opti
             throw new InvalidBlockException(suggestedBlock, error);
         }
 
+        PostValidation(suggestedBlock, block, receipts, options);
+    }
+
+    protected virtual void PostValidation(Block suggestedBlock, Block processedBlock, TxReceipt[] receipts, ProcessingOptions options)
+    {
         // Block is valid, copy the execution artifacts back onto the suggested block.
         // Forward sync suggests blocks without BAL payloads, so the generated BAL needs to
         // follow the suggested block through main-chain updates and persistence.
-        suggestedBlock.AccountChanges = block.AccountChanges;
-        suggestedBlock.ExecutionRequests = block.ExecutionRequests;
-        suggestedBlock.GeneratedBlockAccessList = block.GeneratedBlockAccessList;
-        suggestedBlock.EncodedBlockAccessList = block.EncodedBlockAccessList ?? suggestedBlock.EncodedBlockAccessList;
+        suggestedBlock.AccountChanges = processedBlock.AccountChanges;
+        suggestedBlock.ExecutionRequests = processedBlock.ExecutionRequests;
+        suggestedBlock.GeneratedBlockAccessList = processedBlock.GeneratedBlockAccessList;
+        suggestedBlock.EncodedBlockAccessList = processedBlock.EncodedBlockAccessList ?? suggestedBlock.EncodedBlockAccessList;
     }
 
     protected bool ShouldComputeStateRoot(BlockHeader header) =>

src/Nethermind/Nethermind.Xdc.Test/Helpers/XdcTestDepositContract.cs

@@ -20,9 +20,15 @@ public Address[] GetCandidatesByStake(BlockHeader blockHeader) =>
     public Address[] GetCandidates(BlockHeader blockHeader) =>
         candidateContainer.MasternodeCandidates.Select(m => m.Address).ToArray();
 
+    public Address[] GetCandidates(ITransactionProcessor transactionProcessor, BlockHeader blockHeader) =>
+        GetCandidates(blockHeader);
+
     public UInt256 GetCandidateStake(BlockHeader blockHeader, Address candidate) =>
         10_000_000.Ether;
 
+    public UInt256 GetCandidateStake(ITransactionProcessor transactionProcessor, BlockHeader blockHeader, Address candidate) =>
+        GetCandidateStake(blockHeader, candidate);
+
     public Address GetCandidateOwner(BlockHeader blockHeader, Address candidate) =>
         throw new System.NotImplementedException();
 

src/Nethermind/Nethermind.Xdc.Test/ModuleTests/PenaltyTests.cs

@@ -228,8 +228,8 @@ private static MockedPenaltyContext CreateMockedPenaltyContext(
         xdcSpec.MergeSignRange.Returns((ulong)MergeSignRange);
         xdcSpec.IsTipUpgradePenaltyEnabled.Returns(true);
         xdcSpec.LimitPenaltyEpoch.Returns((ulong)limitPenaltyEpoch);
-        xdcSpec.MinimumSigningTx.Returns(2);
-        xdcSpec.MinimumMinerBlockPerEpoch.Returns(1);
+        xdcSpec.MinimumSigningTx.Returns(2UL);
+        xdcSpec.MinimumMinerBlockPerEpoch.Returns(1UL);
 
         ISpecProvider specProvider = Substitute.For<ISpecProvider>();
         specProvider.GetSpec(Arg.Any<ForkActivation>()).Returns(xdcSpec);

src/Nethermind/Nethermind.Xdc.Test/ModuleTests/RewardTests.cs

@@ -21,6 +21,7 @@
 using Nethermind.Xdc.Test.Helpers;
 using Nethermind.Xdc.Types;
 using NSubstitute;
+using NSubstitute.ExceptionExtensions;
 using NUnit.Framework;
 using System.Collections.Generic;
 using System.Linq;
@@ -91,8 +92,7 @@ public async Task TestHookRewardV2()
             masternodeVotingContract,
             Substitute.For<IMintedRecordContract>(),
             signingTxCache,
-            CreateXdcTransactionProcessor(chain.SpecProvider, masternodeVotingContract, chain.MainWorldState),
-            Substitute.For<IRewardsStore>()
+            CreateXdcTransactionProcessor(chain.SpecProvider, masternodeVotingContract, chain.MainWorldState)
         );
 
         XdcBlockHeader head = (XdcBlockHeader)chain.BlockTree.Head!.Header;
@@ -228,8 +228,7 @@ public async Task TestHookRewardV2SplitReward()
             masternodeVotingContract,
             Substitute.For<IMintedRecordContract>(),
             signingTxCache,
-            CreateXdcTransactionProcessor(chain.SpecProvider, masternodeVotingContract, chain.MainWorldState),
-            Substitute.For<IRewardsStore>()
+            CreateXdcTransactionProcessor(chain.SpecProvider, masternodeVotingContract, chain.MainWorldState)
         );
 
         XdcBlockHeader head = (XdcBlockHeader)chain.BlockTree.Head!.Header;
@@ -408,7 +407,7 @@ public void RewardCalculator_SplitReward_MatchesRounding()
             .Returns(ci => ci.ArgAt<Address>(1));
 
         SigningTxCache signingTxCache = new(tree, specProvider);
-        XdcRewardCalculator rewardCalculator = new(epochSwitchManager, specProvider, tree, votingContract, Substitute.For<IMintedRecordContract>(), signingTxCache, CreateXdcTransactionProcessor(specProvider, votingContract), Substitute.For<IRewardsStore>());
+        XdcRewardCalculator rewardCalculator = new(epochSwitchManager, specProvider, tree, votingContract, Substitute.For<IMintedRecordContract>(), signingTxCache, CreateXdcTransactionProcessor(specProvider, votingContract));
         BlockReward[] rewards = rewardCalculator.CalculateRewards(blocks.Last());
 
         Assert.That(rewards, Has.Length.EqualTo(3));
@@ -534,8 +533,12 @@ public void TestHookRewardAfterUpgrade()
             observer1.Address,
             observer2.Address,
         ];
-        votingContract.GetCandidates(Arg.Any<BlockHeader>()).Returns(rewardCandidates);
-        votingContract.GetCandidateStake(Arg.Any<BlockHeader>(), Arg.Any<Address>()).Returns(UInt256.One);
+        votingContract.GetCandidates(Arg.Any<BlockHeader>())
+            .Throws(new InvalidOperationException("Readonly candidates lookup should not be used for block-processing rewards."));
+        votingContract.GetCandidates(Arg.Any<ITransactionProcessor>(), Arg.Any<BlockHeader>()).Returns(rewardCandidates);
+        votingContract.GetCandidateStake(Arg.Any<BlockHeader>(), Arg.Any<Address>())
+            .Throws(new InvalidOperationException("Readonly stake lookup should not be used for block-processing rewards."));
+        votingContract.GetCandidateStake(Arg.Any<ITransactionProcessor>(), Arg.Any<BlockHeader>(), Arg.Any<Address>()).Returns(UInt256.One);
 
         IWorldState worldState = TestWorldStateFactory.CreateForTest(TestMemDbProvider.Init(), LimboLogs.Instance);
         using IDisposable _ = worldState.BeginScope(IWorldState.PreGenesis);
@@ -556,8 +559,7 @@ public void TestHookRewardAfterUpgrade()
             votingContract,
             mintedRecordContract,
             signingTxCache,
-            transactionProcessor,
-            Substitute.For<IRewardsStore>());
+            transactionProcessor);
 
         BlockReward[] rewards = rewardCalculator.CalculateRewards(blocks[(int)checkpointNumber]);
 
@@ -608,8 +610,7 @@ public void RewardCalculator_CalculateRewardsForSignersAndHolders_MatchesExpecte
             masternodeVotingContract,
             Substitute.For<IMintedRecordContract>(),
             signingTxCache,
-            CreateXdcTransactionProcessor(specProvider, masternodeVotingContract),
-            Substitute.For<IRewardsStore>()
+            CreateXdcTransactionProcessor(specProvider, masternodeVotingContract)
             );
 
         UInt256 totalReward = UInt256.Parse("171000000000000000000");
@@ -647,8 +648,7 @@ public void DistributeRewards_OwnerIsFoundation_MatchesReferenceOverwriteBehavio
             masternodeVotingContract,
             Substitute.For<IMintedRecordContract>(),
             signingTxCache,
-            CreateXdcTransactionProcessor(specProvider, masternodeVotingContract),
-            Substitute.For<IRewardsStore>());
+            CreateXdcTransactionProcessor(specProvider, masternodeVotingContract));
 
         Address signer = new("0x80b329b66ddfe2180904d6ae737283a3f1860b83");
         Address foundationWalletAddr = new("0x5cb041be27deb4a506ad63d082c6043b4a5c6898");
@@ -687,11 +687,9 @@ private static XdcRewardCalculatorSource CreateRewardCalculatorSource(bool isSub
         IMasternodeVotingContract masternodeVotingContract = Substitute.For<IMasternodeVotingContract>();
         IMintedRecordContract mintedRecordContract = Substitute.For<IMintedRecordContract>();
         ISigningTxCache signingTxCache = Substitute.For<ISigningTxCache>();
-        IRewardsStore rewardsStore = Substitute.For<IRewardsStore>();
-
         return isSubnet
-            ? new XdcSubnetRewardCalculatorSource(epochSwitchManager, specProvider, blockTree, masternodeVotingContract, mintedRecordContract, signingTxCache, rewardsStore)
-            : new XdcRewardCalculatorSource(epochSwitchManager, specProvider, blockTree, masternodeVotingContract, mintedRecordContract, signingTxCache, rewardsStore);
+            ? new XdcSubnetRewardCalculatorSource(epochSwitchManager, specProvider, blockTree, masternodeVotingContract, mintedRecordContract, signingTxCache)
+            : new XdcRewardCalculatorSource(epochSwitchManager, specProvider, blockTree, masternodeVotingContract, mintedRecordContract, signingTxCache);
     }
 
     private static XdcTransactionProcessor CreateXdcTransactionProcessor(

src/Nethermind/Nethermind.Xdc.Test/ModuleTests/RpcModuleTests.cs

@@ -829,23 +829,15 @@ public void GetRewardByAccount_ShouldReturnSuccess_WhenRewardsExist()
         ];
 
         _epochSwitchManager.GetEpochSwitchInfoBetween(beginHeader, endHeader).Returns(epochSwitchInfos);
-        _rewardsStore.TryGetRetainedRange(out Arg.Any<ulong>(), out Arg.Any<ulong>())
-            .Returns(callInfo =>
-            {
-                callInfo[0] = epoch1;
-                callInfo[1] = epoch2;
-                return true;
-            });
-
-        _rewardsStore.HasEpochRewards(epoch1).Returns(true);
-        _rewardsStore.HasEpochRewards(epoch2).Returns(true);
-        _rewardsStore.TryGetAccountReward(account, epoch1, out Arg.Any<UInt256>())
+        _rewardsStore.HasEpochRewards(TestItem.KeccakA).Returns(true);
+        _rewardsStore.HasEpochRewards(TestItem.KeccakB).Returns(true);
+        _rewardsStore.TryGetAccountReward(account, TestItem.KeccakA, out Arg.Any<UInt256>())
             .Returns(callInfo =>
             {
                 callInfo[2] = (UInt256)10;
                 return true;
             });
-        _rewardsStore.TryGetAccountReward(account, epoch2, out Arg.Any<UInt256>())
+        _rewardsStore.TryGetAccountReward(account, TestItem.KeccakB, out Arg.Any<UInt256>())
             .Returns(callInfo =>
             {
                 callInfo[2] = (UInt256)20;
@@ -865,45 +857,6 @@ public void GetRewardByAccount_ShouldReturnSuccess_WhenRewardsExist()
         Assert.That(result.Data.Total.TotalAccountReward, Is.EqualTo((UInt256)30));
     }
 
-    [Test]
-    public void GetRewardByAccount_ShouldReturnFail_WhenRequestIsPruned()
-    {
-        // Arrange
-        Address account = TestItem.AddressA;
-        const ulong begin = 100;
-        const ulong end = 200;
-        const ulong requestedEpoch = 120;
-        const ulong oldestRetained = 150;
-        const ulong newestRetained = 300;
-
-        XdcBlockHeader beginHeader = Build.A.XdcBlockHeader().WithNumber(begin).TestObject;
-        XdcBlockHeader endHeader = Build.A.XdcBlockHeader().WithNumber(end).TestObject;
-
-        _blockTree.FindHeader(begin).Returns(beginHeader);
-        _blockTree.FindHeader(end).Returns(endHeader);
-
-        EpochSwitchInfo[] epochSwitchInfos =
-        [
-            new EpochSwitchInfo(Array.Empty<Address>(), Array.Empty<Address>(), Array.Empty<Address>(), new BlockRoundInfo(TestItem.KeccakA, 1, (long)requestedEpoch)),
-        ];
-
-        _epochSwitchManager.GetEpochSwitchInfoBetween(beginHeader, endHeader).Returns(epochSwitchInfos);
-        _rewardsStore.TryGetRetainedRange(out Arg.Any<ulong>(), out Arg.Any<ulong>())
-            .Returns(callInfo =>
-            {
-                callInfo[0] = oldestRetained;
-                callInfo[1] = newestRetained;
-                return true;
-            });
-
-        // Act
-        ResultWrapper<AccountRewardResponse> result = _rpcModule.GetRewardByAccount(account, begin, end);
-
-        // Assert
-        Assert.That(result.Result, Is.Not.EqualTo(Result.Success));
-        Assert.That(result.ErrorCode, Is.EqualTo(ErrorCodes.PrunedHistoryUnavailable));
-    }
-
     [Test]
     public void GetRewardByAccount_ShouldReturnFail_WhenRewardsMissingForEpoch()
     {
@@ -925,8 +878,7 @@ public void GetRewardByAccount_ShouldReturnFail_WhenRewardsMissingForEpoch()
         ];
 
         _epochSwitchManager.GetEpochSwitchInfoBetween(beginHeader, endHeader).Returns(epochSwitchInfos);
-        _rewardsStore.TryGetRetainedRange(out Arg.Any<ulong>(), out Arg.Any<ulong>()).Returns(false);
-        _rewardsStore.HasEpochRewards(epoch).Returns(false);
+        _rewardsStore.HasEpochRewards(TestItem.KeccakA).Returns(false);
 
         // Act
         ResultWrapper<AccountRewardResponse> result = _rpcModule.GetRewardByAccount(account, begin, end);