Merge pull request #1033 from HorizenOfficial/st/stakeV2_fixes

St/stake v2 fixes

qa/SidechainTestFramework/account/smart_contract_resources/contracts/ForgerStakesV2.sol

@@ -10,8 +10,8 @@ interface ForgerStakesV2 {
     // Event declaration
     // Up to 3 parameters can be indexed.
     // Indexed parameters help you filter the logs by the indexed parameter
-    event RegisterForger(address  sender, bytes32 indexed signPubKey, bytes32 indexed vrf1, bytes1 indexed vrf2, uint256 value, uint32 rewardShare, address reward_address);
-    event UpdateForger(address indexed sender, bytes32 signPubKey,  bytes32 indexed vrf1, bytes1 indexed vrf2,  uint32 rewardShare, address reward_address);
+    event RegisterForger(address indexed sender, bytes32 signPubKey, bytes32 indexed vrf1, bytes1 indexed vrf2, uint256 value, uint32 rewardShare, address reward_address);
+    event UpdateForger(address indexed sender, bytes32 signPubKey, bytes32 indexed vrf1, bytes1 indexed vrf2, uint32 rewardShare, address reward_address);
     event DelegateForgerStake(address indexed sender, bytes32 signPubKey, bytes32 indexed vrf1, bytes1 indexed vrf2, uint256 value);
     event WithdrawForgerStake(address indexed sender, bytes32 signPubKey, bytes32 indexed vrf1, bytes1 indexed vrf2, uint256 value);
     event ActivateStakeV2();
@@ -121,13 +121,15 @@ interface ForgerStakesV2 {
       Returns the paginated list of stakes delegated to a specific forger, grouped by delegator address.
       Each element of the list is the total amount delegated by a specific address.
       nextIndex will contain the index of the next element not returned yet. If no element is still present, next will be -1.
+      The returned array length may be less than pageSize even if there are still additional elements because stakes with 0 amount are filtered out.
     */
     function getPagedForgersStakesByForger(bytes32 signPubKey, bytes32 vrf1, bytes1 vrf2, int32 startIndex, int32 pageSize) external view returns (int32 nextIndex, StakeDataDelegator[] memory listOfDelegatorStakes);
 
     /*
       Returns the paginated list of stakes delegated by a specific address, grouped by forger.
       Each element of the list is the total amount delegated to  a specific forger.
       nextIndex will contain the index of the next element not returned yet. If no element is still present, next will be -1.
+      The returned array length may be less than pageSize even if there are still additional elements because stakes with 0 amount are filtered out.
     */
     function getPagedForgersStakesByDelegator(address delegator, int32 startIndex, int32 pageSize) external view returns (int32 nextIndex, StakeDataForger[] memory listOfForgerStakes);
 

qa/sc_evm_forger_v2_register.py

@@ -1,5 +1,6 @@
 #!/usr/bin/env python3
 import time
+import logging
 from decimal import Decimal
 
 from eth_abi import decode
@@ -43,6 +44,9 @@ def __init__(self):
                          block_timestamp_rewind=1500 * EVM_APP_SLOT_TIME * VERSION_1_3_FORK_EPOCH)
 
     def run_test(self):
+        if self.options.all_forks:
+            logging.info("This test cannot be executed with --allforks")
+            exit()
 
         mc_node = self.nodes[0]
         sc_node_1 = self.sc_nodes[0]
@@ -530,22 +534,24 @@ def check_register_event(register_forger_event, sender, vrf_pub_key, block_sign_
             event_signature_to_log_topic('RegisterForger(address,bytes32,bytes32,bytes1,uint256,uint32,address)')))
     assert_equal(event_signature, event_id, "Wrong event signature in topics")
 
-    pubKey25519 = decode(['bytes32'], hex_str_to_bytes(register_forger_event['topics'][1][2:]))[0]
-    assert_equal(block_sign_pub_key, bytes_to_hex_str(pubKey25519), "Wrong from address in topics")
+    from_addr = decode(['address'], hex_str_to_bytes(register_forger_event['topics'][1][2:]))[0][2:]
+    assert_equal(sender.lower(), from_addr.lower(), "Wrong from address in topics")
 
     vrf1 = decode(['bytes32'], hex_str_to_bytes(register_forger_event['topics'][2][2:]))[0]
     vrf2 = decode(['bytes1'], hex_str_to_bytes(register_forger_event['topics'][3][2:]))[0]
     assert_equal(vrf_pub_key,
                  bytes_to_hex_str(vrf1) + bytes_to_hex_str(vrf2), "wrong vrfPublicKey")
 
-    (from_addr, value, share, reward_contract_address) = decode(['address', 'uint256', 'uint32', 'address'],
-                                                                hex_str_to_bytes(register_forger_event['data'][2:]))
-    assert_equal(to_checksum_address(sender), to_checksum_address(from_addr), "Wrong sender in event")
+    (pubKey25519, value, share, reward_contract_address) = decode(['bytes32', 'uint256', 'uint32', 'address'],
+                                                         hex_str_to_bytes(register_forger_event['data'][2:]))
+
+    assert_equal(block_sign_pub_key, bytes_to_hex_str(pubKey25519), "Wrong from address in topics")
     assert_equal(staked_amount, value, "Wrong amount in event")
     assert_equal(rewards_share, share, "Wrong rewards_share in event")
     assert_equal(reward_address, reward_address, "Wrong reward_address in event")
 
 
+
 def check_update_event(update_forger_event, sender, vrf_pub_key, block_sign_pub_key, rewards_share,
                        reward_address):
     assert_equal(4, len(update_forger_event['topics']), "Wrong number of topics in update forger event")

qa/sc_evm_native_forger_v2.py

@@ -29,7 +29,6 @@
 NULL_ADDRESS = '0x0000000000000000000000000000000000000000'
 
 """
-If it is run with --allforks, all the existing forks are enabled at epoch 2, so it will use Shanghai EVM.
 Configuration: 
     - 2 SC nodes connected with each other
     - 1 MC node
@@ -58,6 +57,9 @@ def __init__(self):
                          block_timestamp_rewind=1500 * EVM_APP_SLOT_TIME * VERSION_1_4_FORK_EPOCH)
 
     def run_test(self):
+        if self.options.all_forks:
+            logging.info("This test cannot be executed with --allforks")
+            exit()
 
         mc_node = self.nodes[0]
         sc_node_1 = self.sc_nodes[0]

sdk/src/main/scala/io/horizen/account/state/ForgerStakeV2MsgProcessor.scala

@@ -430,7 +430,7 @@ object ForgerStakeV2MsgProcessor extends NativeSmartContractWithFork  with Forge
 
     val response = Try {
       // We must trap any exception arising and return the execution reverted exception
-      getPagedForgersStakesByDelegator(view, cmdInput.delegator, cmdInput.startIndex, cmdInput.pageSize)
+      StakeStorage.getPagedForgersStakesByDelegator(view, cmdInput.delegator, cmdInput.startIndex, cmdInput.pageSize)
     } match {
       case Success(response) => response
       case Failure(ex) =>
@@ -450,7 +450,7 @@ object ForgerStakeV2MsgProcessor extends NativeSmartContractWithFork  with Forge
 
     val response = Try {
       // We must trap any exception arising and return the execution reverted exception
-      getPagedForgersStakesByForger(view, cmdInput.forgerPublicKeys, cmdInput.startIndex, cmdInput.pageSize)
+      StakeStorage.getPagedForgersStakesByForger(view, cmdInput.forgerPublicKeys, cmdInput.startIndex, cmdInput.pageSize)
     } match {
       case Success(response) => response
       case Failure(ex) =>
@@ -459,18 +459,9 @@ object ForgerStakeV2MsgProcessor extends NativeSmartContractWithFork  with Forge
     PagedForgersStakesByForgerOutput(response.nextStartPos, response.stakesData).encode()
   }
 
-
-  override def getPagedForgersStakesByForger(view: BaseAccountStateView, forger: ForgerPublicKeys, startPos: Int, pageSize: Int): PagedStakesByForgerResponse = {
-    StakeStorage.getPagedForgersStakesByForger(view, forger, startPos, pageSize)
-  }
-
-  override def getPagedForgersStakesByDelegator(view: BaseAccountStateView, delegator: Address, startPos: Int, pageSize: Int): PagedStakesByDelegatorResponse = {
-    StakeStorage.getPagedForgersStakesByDelegator(view, delegator, startPos, pageSize)
-  }
-
   def doGetForgerCmd(invocation: Invocation, view: BaseAccountStateView): Array[Byte] = {
-    checkForgerStakesV2IsActive(view)
     requireIsNotPayable(invocation)
+    checkForgerStakesV2IsActive(view)
 
     val inputParams = getArgumentsFromData(invocation.input)
     val cmdInput = SelectByForgerCmdInputDecoder.decode(inputParams)
@@ -483,9 +474,8 @@ object ForgerStakeV2MsgProcessor extends NativeSmartContractWithFork  with Forge
   }
 
   def doGetPagedForgersCmd(invocation: Invocation, view: BaseAccountStateView): Array[Byte] = {
-
-    checkForgerStakesV2IsActive(view)
     requireIsNotPayable(invocation)
+    checkForgerStakesV2IsActive(view)
 
     val inputParams = getArgumentsFromData(invocation.input)
     val PagedForgersCmdInput(startPos, pageSize) = PagedForgersCmdInputDecoder.decode(inputParams)
@@ -495,8 +485,8 @@ object ForgerStakeV2MsgProcessor extends NativeSmartContractWithFork  with Forge
   }
 
   def doGetCurrentConsensusEpochCmd(invocation: Invocation, view: BaseAccountStateView, context: ExecutionContext): Array[Byte] = {
-    checkForgerStakesV2IsActive(view)
     requireIsNotPayable(invocation)
+    checkForgerStakesV2IsActive(view)
     checkInputDoesntContainParams(invocation)
 
     ConsensusEpochCmdOutput(context.blockContext.consensusEpochNumber).encode()
@@ -610,6 +600,16 @@ object ForgerStakeV2MsgProcessor extends NativeSmartContractWithFork  with Forge
     StakeStorage.getAllForgerStakes(view)
   }
 
+  override private[horizen] def getPagedForgersStakesByForger(view: BaseAccountStateView, forger: ForgerPublicKeys, startPos: Int, pageSize: Int): PagedStakesByForgerResponse = {
+    checkForgerStakesV2IsActive(view)
+    StakeStorage.getPagedForgersStakesByForger(view, forger, startPos, pageSize)
+  }
+
+  override private[horizen] def getPagedForgersStakesByDelegator(view: BaseAccountStateView, delegator: Address, startPos: Int, pageSize: Int): PagedStakesByDelegatorResponse = {
+    checkForgerStakesV2IsActive(view)
+    StakeStorage.getPagedForgersStakesByDelegator(view, delegator, startPos, pageSize)
+  }
+
   override private[horizen] def getForgingStakes(view: BaseAccountStateView): Seq[ForgingStakeInfo] = {
     StakeStorage.getForgingStakes(view)
   }
@@ -619,4 +619,7 @@ object ForgerStakeV2MsgProcessor extends NativeSmartContractWithFork  with Forge
   }
 
   override private[horizen] def isActive(view: BaseAccountStateView): Boolean = StakeStorage.isActive(view)
+
+
+
 }

sdk/src/main/scala/io/horizen/account/state/nativescdata/forgerstakev2/StakeStorage.scala

@@ -253,12 +253,6 @@ object StakeStorage {
   }
 
   def getPagedForgersStakesByForger(view: BaseAccountStateView, forger: ForgerPublicKeys, startPos: Int, pageSize: Int): PagedStakesByForgerResponse = {
-
-    if (!StakeStorage.isActive(view)) {
-      val msgStr = s"Forger stake V2 is not activated"
-      throw new IllegalArgumentException(msgStr)
-    }
-
     if (startPos < 0)
       throw new IllegalArgumentException(s"Negative start position: $startPos can not be negative")
     if (pageSize <= 0)
@@ -293,11 +287,6 @@ object StakeStorage {
   }
 
   def getPagedForgersStakesByDelegator(view: BaseAccountStateView,  delegator: Address, startPos: Int, pageSize: Int): PagedStakesByDelegatorResponse = {
-
-    if (!StakeStorage.isActive(view)) {
-      val msgStr = s"Forger stake V2 is not activated"
-      throw new IllegalArgumentException(msgStr)
-    }
     if (startPos < 0)
       throw new IllegalArgumentException(s"Negative start position: $startPos")
     if (pageSize <= 0)
@@ -318,9 +307,9 @@ object StakeStorage {
 
     val resultList = (startPos until endPos).view.map(index => {
       val forgerKey = listOfForgers.getForgerKey(view, index)
+      val forger = ForgerMap.getForgerOption(view, forgerKey).getOrElse(throw new ExecutionRevertedException("Forger doesn't exist."))
       val stakeHistory = StakeHistory(forgerKey, delegatorKey)
       val amount = stakeHistory.getLatestAmount(view)
-      val forger = ForgerMap.getForgerOption(view, forgerKey).getOrElse(throw new ExecutionRevertedException("Forger doesn't exist."))
       StakeDataForger(forger.forgerPublicKeys, amount)
     }).filter(_.stakedAmount.signum() > 0).toList
 
@@ -346,7 +335,7 @@ object StakeStorage {
         val lastCheckpoint = getCheckpoint(view, lastElemIndex)
         val newAmount = op(lastCheckpoint.stakedAmount)
         if (lastCheckpoint.fromEpochNumber == epoch) {
-          // Let's check if the newAmount is the same og the previous checkpoint. In that case, this last checkpoint
+          // Let's check if the newAmount is the same of the previous checkpoint. In that case, this last checkpoint
           // is removed.
           val secondLastCheckpointIdx = lastElemIndex - 1
           if (secondLastCheckpointIdx > -1 && getCheckpoint(view, secondLastCheckpointIdx).stakedAmount == newAmount) {

sdk/src/main/scala/io/horizen/account/state/nativescdata/forgerstakev2/events/RegisterForger.scala

@@ -10,8 +10,8 @@ import java.math.BigInteger
 import scala.annotation.meta.getter
 
 case class RegisterForger(
-    @(Parameter @getter)(1) sender: AbiAddress,
-    @(Parameter @getter)(2) @(Indexed @getter) signPubKey: Bytes32,
+    @(Parameter @getter)(1) @(Indexed @getter) sender: AbiAddress,
+    @(Parameter @getter)(2) signPubKey: Bytes32,
     @(Parameter @getter)(3) @(Indexed @getter) vrf1: Bytes32,
     @(Parameter @getter)(4) @(Indexed @getter) vrf2: Bytes1,
     @(Parameter @getter)(5) value: Uint256,

sdk/src/main/scala/io/horizen/account/utils/AccountFeePaymentsUtils.scala

@@ -70,17 +70,18 @@ object AccountFeePaymentsUtils {
     forgerInfo: ForgerInfo,
   ): (AccountPayment, Option[DelegatorFeePayment]) = {
     val forgerRewardAddress = feePayment.identifier.getAddress
-    val delegatorShare = BigInteger.valueOf(forgerInfo.rewardShare)
     val totalMcReward = feePayment.valueFromMainchain
 
-    if (delegatorShare.compareTo(BigInteger.ZERO) == 0) {
+    if (forgerInfo.rewardShare == 0) {
       // No delegator share, all reward goes to the forger
       val totalFeeReward = feePayment.value.subtract(totalMcReward)
       val forgerPayment =
         AccountPayment(forgerRewardAddress, feePayment.value, Some(totalMcReward), Some(totalFeeReward))
       return (forgerPayment, None)
     }
 
+    val delegatorShare = BigInteger.valueOf(forgerInfo.rewardShare)
+
     val delegatorReward = feePayment.value.multiply(delegatorShare).divide(TOTAL_SHARE)
     val delegatorMcReward = totalMcReward.multiply(delegatorShare).divide(TOTAL_SHARE)
     val delegatorFeeReward = delegatorReward.subtract(delegatorMcReward)

sdk/src/test/scala/io/horizen/account/AccountSidechainNodeViewHolderTest.scala

@@ -500,7 +500,7 @@ class AccountSidechainNodeViewHolderTest extends JUnitSuite
       case m =>
         m match {
           case ModifiersProcessingResult(applied, cleared) => {
-            assertTrue("Applied block sequence is differ", applied.toSet.equals(correctSequence.toSet))
+            assertEquals("Applied block sequence is differ", correctSequence.toSet, applied.toSet)
             assertTrue("Cleared block sequence is not empty.", cleared.isEmpty)
             true
           }

sdk/src/test/scala/io/horizen/account/state/ForgerStakeV2MsgProcessorTest.scala

@@ -2079,18 +2079,18 @@ class ForgerStakeV2MsgProcessorTest
     assertEquals("Wrong address", contractAddress, actualEvent.address)
     assertEquals("Wrong number of topics", NumOfIndexedRegisterForgerEvtParams + 1, actualEvent.topics.length) //The first topic is the hash of the signature of the event
     assertArrayEquals("Wrong event signature", RegisterForgerEventSig, actualEvent.topics(0).toBytes)
-    assertEquals("Wrong signer key address in topic", expectedEvent.signPubKey, decodeEventTopic(actualEvent.topics(1), TypeReference.makeTypeReference(expectedEvent.signPubKey.getTypeAsString)))
+    assertEquals("Wrong sender address in topic", expectedEvent.sender, decodeEventTopic(actualEvent.topics(1), TypeReference.makeTypeReference(expectedEvent.sender.getTypeAsString)))
     assertEquals("Wrong vrfKey1 in topic", expectedEvent.vrf1, decodeEventTopic(actualEvent.topics(2), TypeReference.makeTypeReference(expectedEvent.vrf1.getTypeAsString)))
     assertEquals("Wrong vrfKey2 in topic", expectedEvent.vrf2, decodeEventTopic(actualEvent.topics(3), TypeReference.makeTypeReference(expectedEvent.vrf2.getTypeAsString)))
 
     val listOfRefs = util.Arrays.asList(
-      TypeReference.makeTypeReference(expectedEvent.sender.getTypeAsString),
+      TypeReference.makeTypeReference(expectedEvent.signPubKey.getTypeAsString),
       TypeReference.makeTypeReference(expectedEvent.value.getTypeAsString),
       TypeReference.makeTypeReference(expectedEvent.rewardShare.getTypeAsString),
       TypeReference.makeTypeReference(expectedEvent.rewardAddress.getTypeAsString))
       .asInstanceOf[util.List[TypeReference[Type[_]]]]
     val listOfDecodedData = FunctionReturnDecoder.decode(BytesUtils.toHexString(actualEvent.data), listOfRefs)
-    assertEquals("Wrong sender in data", expectedEvent.sender, listOfDecodedData.get(0))
+    assertEquals("Wrong signer key in data", expectedEvent.signPubKey, listOfDecodedData.get(0))
     assertEquals("Wrong amount in data", expectedEvent.value.getValue, listOfDecodedData.get(1).getValue)
     assertEquals("Wrong reward share in data", expectedEvent.rewardShare.getValue, listOfDecodedData.get(2).getValue)
     assertEquals("Wrong reward address in data", expectedEvent.rewardAddress, listOfDecodedData.get(3))