Fixing tests and refactoring tx cost calculation

rsksmart · Jun 15, 2023 · 990e215 · 990e215
1 parent 9923d11
commit 990e215
Show file tree

Hide file tree

Showing 13 changed files with 68 additions and 9 deletions.
diff --git a/rskj-core/src/main/java/org/ethereum/config/blockchain/upgrades/ConsensusRule.java b/rskj-core/src/main/java/org/ethereum/config/blockchain/upgrades/ConsensusRule.java
@@ -85,6 +85,7 @@ public enum ConsensusRule {
     RSKIP377("rskip377"),
     RSKIP383("rskip383"),
     RSKIP385("rskip385"),
+    RSKIPXXX("rskipXXX"), // From EIP-2028 calldata gas reduction
     ;
 
     private String configKey;

diff --git a/rskj-core/src/main/java/org/ethereum/config/blockchain/upgrades/NetworkUpgrade.java b/rskj-core/src/main/java/org/ethereum/config/blockchain/upgrades/NetworkUpgrade.java
@@ -31,7 +31,8 @@ public enum NetworkUpgrade {
     HOP400("hop400"),
     HOP401("hop401"),
     FINGERROOT500("fingerroot500"),
-    TBD600("tbd600");
+    TBD600("tbd600"),
+    XXX("xxx");
 
     private String name;
 

diff --git a/rskj-core/src/main/java/org/ethereum/core/Transaction.java b/rskj-core/src/main/java/org/ethereum/core/Transaction.java
@@ -205,7 +205,11 @@ public long transactionCost(Constants constants, ActivationConfig.ForBlock activ
         long nonZeroes = this.nonZeroDataBytes();
         long zeroVals = ListArrayUtil.getLength(this.getData()) - nonZeroes;
 
-        return (this.isContractCreation() ? GasCost.TRANSACTION_CREATE_CONTRACT : GasCost.TRANSACTION) + zeroVals * GasCost.TX_ZERO_DATA + nonZeroes * getTxNoZeroData(activations);
+        long transactionCost =  this.isContractCreation() ? GasCost.TRANSACTION_CREATE_CONTRACT : GasCost.TRANSACTION;
+
+        long transactionZeroData = getTxNoZeroData(activations);
+
+        return transactionCost + zeroVals * GasCost.TX_ZERO_DATA + nonZeroes * transactionZeroData;
     }
 
     private static long getTxNoZeroData(ActivationConfig.ForBlock activations) {

diff --git a/rskj-core/src/main/resources/config/devnet.conf b/rskj-core/src/main/resources/config/devnet.conf
@@ -12,7 +12,8 @@ blockchain.config {
         hop400 = 0,
         hop401 = 0,
         fingerroot500 = 0,
-        tbd600 = 0
+        tbd600 = -1
+        xxx = -1
     },
     consensusRules = {
         rskip97 = -1 # disable orchid difficulty drop

diff --git a/rskj-core/src/main/resources/config/main.conf b/rskj-core/src/main/resources/config/main.conf
@@ -13,6 +13,7 @@ blockchain.config {
         hop401 = 4976300,
         fingerroot500 = 5468000,
         tbd600 = -1
+        xxx = -1
     }
 }
 

diff --git a/rskj-core/src/main/resources/config/regtest.conf b/rskj-core/src/main/resources/config/regtest.conf
@@ -12,7 +12,8 @@ blockchain.config {
         hop400 = 0,
         hop401 = 0,
         fingerroot500 = 0
-        tbd600 = 0
+        tbd600 = -1
+        xxx = -1
     },
     consensusRules = {
         rskip97 = -1 # disable orchid difficulty drop

diff --git a/rskj-core/src/main/resources/config/testnet.conf b/rskj-core/src/main/resources/config/testnet.conf
@@ -13,6 +13,7 @@ blockchain.config {
         hop401 = 3362200,
         fingerroot500 = 4015800,
         tbd600 = -1
+        xxx = -1
     },
     consensusRules = {
         rskip97 = -1, # disable orchid difficulty drop

diff --git a/rskj-core/src/main/resources/expected.conf b/rskj-core/src/main/resources/expected.conf
@@ -84,6 +84,7 @@ blockchain = {
              rskip377 = <hardforkName>
              rskip383 = <hardforkName>
              rskip385 = <hardforkName>
+             rskipXXX = <hardforkName>
          }
     }
     gc = {

diff --git a/rskj-core/src/main/resources/reference.conf b/rskj-core/src/main/resources/reference.conf
@@ -71,6 +71,7 @@ blockchain = {
             rskip377 = fingerroot500
             rskip383 = fingerroot500
             rskip385 = fingerroot500
+            rskipXXX = tbd600
         }
     }
     gc = {

diff --git a/rskj-core/src/test/java/co/rsk/mine/TransactionModuleTest.java b/rskj-core/src/test/java/co/rsk/mine/TransactionModuleTest.java
@@ -47,6 +47,9 @@
 import co.rsk.validators.ProofOfWorkRule;
 import org.bouncycastle.util.BigIntegers;
 import org.bouncycastle.util.encoders.Hex;
+import org.ethereum.config.Constants;
+import org.ethereum.config.blockchain.upgrades.ActivationConfig;
+import org.ethereum.config.blockchain.upgrades.ConsensusRule;
 import org.ethereum.core.*;
 import org.ethereum.crypto.ECKey;
 import org.ethereum.crypto.HashUtil;
@@ -74,19 +77,62 @@
 import org.ethereum.vm.PrecompiledContracts;
 import org.ethereum.vm.program.invoke.ProgramInvokeFactoryImpl;
 import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.mockito.Mockito;
 
 import java.math.BigInteger;
 import java.time.Clock;
 
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
 
 class TransactionModuleTest {
-    private final TestSystemProperties config = new TestSystemProperties();
-    private final BlockFactory blockFactory = new BlockFactory(config.getActivationConfig());
-    private final SignatureCache signatureCache = new BlockTxSignatureCache(new ReceivedTxSignatureCache());
+    private TestSystemProperties config;
+    private BlockFactory blockFactory;
+    private SignatureCache signatureCache;
     private TransactionExecutorFactory transactionExecutorFactory;
+    private ActivationConfig activationConfig;
+    private Constants constants;
+    private Block executionBlock;
+
+    @BeforeEach
+    void setUp() {
+        executionBlock = Mockito.mock(Block.class);
+        constants = Mockito.mock(Constants.class);
+        config = Mockito.spy(new TestSystemProperties());
+        activationConfig = Mockito.spy(config.getActivationConfig());
+
+        Mockito.when(config.getActivationConfig()).thenReturn(activationConfig);
+
+        blockFactory = new BlockFactory(activationConfig);
+        signatureCache = new BlockTxSignatureCache(new ReceivedTxSignatureCache());
+
+        Mockito.when(executionBlock.getNumber()).thenReturn(10L);
+    }
+
+    @Test
+    void testTransactionCostWithRSKIPXXXDisabled() {
+        when(executionBlock.getGasLimit()).thenReturn(BigInteger.valueOf(6800000).toByteArray());
+
+        byte[] bytes = new byte[]{-8, 96, -128, 8, -126, -61, 80, -108, -31, 126, -117, -65, -39, -94, 75, -27, 104, -101, 13, -118, 50, 8, 31, -83, -40, -94, 59, 107, 7, -127, -1, 102, -96, -63, -110, 91, -2, 42, -19, 18, 4, 67, -64, 48, -45, -85, -123, 41, 14, -48, -124, 118, 21, -63, -39, -45, 67, 116, -103, 93, 37, 4, 88, -61, 49, -96, 77, -30, -116, 59, -58, -82, -95, 76, 46, 124, 115, -32, -80, 125, 30, -42, -75, -111, -49, -41, 121, -73, -121, -68, -41, 72, -120, 94, 82, 42, 17, 61};
+        Transaction txInBlock = new ImmutableTransaction(bytes);
+
+        Assertions.assertEquals(txInBlock.transactionCost(constants, activationConfig.forBlock(executionBlock.getNumber()), new BlockTxSignatureCache(new ReceivedTxSignatureCache())), 21068L);
+    }
+
+    @Test
+    void testTransactionCostWithRSKIPXXXEnabled() {
+        when(executionBlock.getGasLimit()).thenReturn(BigInteger.valueOf(6800000).toByteArray());
+
+        byte[] bytes = new byte[]{-8, 96, -128, 8, -126, -61, 80, -108, -31, 126, -117, -65, -39, -94, 75, -27, 104, -101, 13, -118, 50, 8, 31, -83, -40, -94, 59, 107, 7, -127, -1, 102, -96, -63, -110, 91, -2, 42, -19, 18, 4, 67, -64, 48, -45, -85, -123, 41, 14, -48, -124, 118, 21, -63, -39, -45, 67, 116, -103, 93, 37, 4, 88, -61, 49, -96, 77, -30, -116, 59, -58, -82, -95, 76, 46, 124, 115, -32, -80, 125, 30, -42, -75, -111, -49, -41, 121, -73, -121, -68, -41, 72, -120, 94, 82, 42, 17, 61};
+        Transaction txInBlock = new ImmutableTransaction(bytes);
+
+        Mockito.doReturn(true)
+                .when(activationConfig).isActive(Mockito.eq(ConsensusRule.RSKIPXXX), Mockito.anyLong());
+
+        Assertions.assertEquals(txInBlock.transactionCost(constants, activationConfig.forBlock(executionBlock.getNumber()), new BlockTxSignatureCache(new ReceivedTxSignatureCache())), 21016L);
+    }
 
     @Test
     void sendTransactionMustNotBeMined() {

diff --git a/...c/test/java/co/rsk/net/handler/txvalidator/TxValidatorIntrinsicGasLimitValidatorTest.java b/...c/test/java/co/rsk/net/handler/txvalidator/TxValidatorIntrinsicGasLimitValidatorTest.java
@@ -43,7 +43,7 @@ class TxValidatorIntrinsicGasLimitValidatorTest {
     @BeforeEach
     void setUp() {
         constants = Constants.regtest();
-        activationConfig = ActivationConfigsForTest.allBut(ConsensusRule.ARE_BRIDGE_TXS_PAID);
+        activationConfig = ActivationConfigsForTest.allBut(ConsensusRule.ARE_BRIDGE_TXS_PAID, ConsensusRule.RSKIPXXX);
     }
 
     @Test

diff --git a/rskj-core/src/test/java/co/rsk/remasc/RemascProcessMinerFeesTest.java b/rskj-core/src/test/java/co/rsk/remasc/RemascProcessMinerFeesTest.java
@@ -87,7 +87,7 @@ class RemascProcessMinerFeesTest {
     @BeforeAll
      static void setUpBeforeClass() {
         config = spy(new TestSystemProperties());
-        when(config.getActivationConfig()).thenReturn(ActivationConfigsForTest.allBut(ConsensusRule.RSKIP85));
+        when(config.getActivationConfig()).thenReturn(ActivationConfigsForTest.allBut(ConsensusRule.RSKIP85, ConsensusRule.RSKIPXXX));
         remascConfig = new RemascConfigFactory(RemascContract.REMASC_CONFIG).createRemascConfig("regtest");
 
         accountsAddressesUpToD = new LinkedList<>();

diff --git a/rskj-core/src/test/java/org/ethereum/config/blockchain/upgrades/ActivationConfigTest.java b/rskj-core/src/test/java/org/ethereum/config/blockchain/upgrades/ActivationConfigTest.java
@@ -111,6 +111,7 @@ class ActivationConfigTest {
             "    rskip377: fingerroot500",
             "    rskip383: fingerroot500",
             "    rskip385: fingerroot500",
+            "    rskipXXX: tbd600",
             "}"
     ));