From bbb4da1a5e52fcd0f1a957247bf36c5ca05be37c Mon Sep 17 00:00:00 2001 From: nathanieliov Date: Thu, 25 Jan 2024 21:26:42 -0400 Subject: [PATCH 01/18] Added RSKIP415 activation code --- .../org/ethereum/config/blockchain/upgrades/ConsensusRule.java | 1 + rskj-core/src/main/resources/expected.conf | 1 + rskj-core/src/main/resources/reference.conf | 1 + .../config/blockchain/upgrades/ActivationConfigTest.java | 1 + 4 files changed, 4 insertions(+) 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 index 5ae8580a070..9ce0be35761 100644 --- 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 @@ -91,6 +91,7 @@ public enum ConsensusRule { RSKIP398("rskip398"), RSKIP400("rskip400"), // From EIP-2028 calldata gas cost reduction RSKIP412("rskip412"), // From EIP-3198 BASEFEE opcode + RSKIP415("rskip415"), ; private String configKey; diff --git a/rskj-core/src/main/resources/expected.conf b/rskj-core/src/main/resources/expected.conf index dec928794e5..6647e73e22e 100644 --- a/rskj-core/src/main/resources/expected.conf +++ b/rskj-core/src/main/resources/expected.conf @@ -90,6 +90,7 @@ blockchain = { rskip398 = rskip400 = rskip412 = + rskip415 = } } gc = { diff --git a/rskj-core/src/main/resources/reference.conf b/rskj-core/src/main/resources/reference.conf index 7fb2ae06c6b..00da71ef4b3 100644 --- a/rskj-core/src/main/resources/reference.conf +++ b/rskj-core/src/main/resources/reference.conf @@ -77,6 +77,7 @@ blockchain = { rskip398 = arrowhead600 rskip400 = arrowhead600 rskip412 = arrowhead600 + rskip415 = arrowhead600 } } gc = { 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 index 1ab4a39d9cb..694fac377bf 100644 --- 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 @@ -117,6 +117,7 @@ class ActivationConfigTest { " rskip398: arrowhead600", " rskip400: arrowhead600", " rskip412: arrowhead600", + " rskip415: arrowhead600", "}" )); From afdb194127e8e3684bdf0c1b8c46a13220fcf1cf Mon Sep 17 00:00:00 2001 From: nathanieliov Date: Mon, 29 Jan 2024 23:33:38 -0400 Subject: [PATCH 02/18] Added getMemberByBtcPublicKey new method --- .../src/main/java/co/rsk/peg/Federation.java | 4 ++++ .../co/rsk/peg/LegacyErpFederationTest.java | 23 ++++++++++++++++++- .../co/rsk/peg/P2shErpFederationTest.java | 22 ++++++++++++++++++ .../peg/StandardMultisigFederationTest.java | 21 +++++++++++++++++ 4 files changed, 69 insertions(+), 1 deletion(-) diff --git a/rskj-core/src/main/java/co/rsk/peg/Federation.java b/rskj-core/src/main/java/co/rsk/peg/Federation.java index 247554623c1..0ad41d9cc50 100644 --- a/rskj-core/src/main/java/co/rsk/peg/Federation.java +++ b/rskj-core/src/main/java/co/rsk/peg/Federation.java @@ -64,6 +64,10 @@ public List getMembers() { return members; } + public Optional getMemberByBtcPublicKey(BtcECKey btcPublicKey) { + return members.stream().filter(federationMember -> federationMember.getBtcPublicKey().equals(btcPublicKey)).findFirst(); + } + public List getBtcPublicKeys() { // Copy instances since we don't control // immutability of BtcECKey instances diff --git a/rskj-core/src/test/java/co/rsk/peg/LegacyErpFederationTest.java b/rskj-core/src/test/java/co/rsk/peg/LegacyErpFederationTest.java index e62b900e596..2a6c929186f 100644 --- a/rskj-core/src/test/java/co/rsk/peg/LegacyErpFederationTest.java +++ b/rskj-core/src/test/java/co/rsk/peg/LegacyErpFederationTest.java @@ -14,7 +14,6 @@ import co.rsk.bitcoinj.core.NetworkParameters; import co.rsk.bitcoinj.core.ScriptException; import co.rsk.bitcoinj.core.Utils; -import co.rsk.bitcoinj.script.ErpFederationRedeemScriptParser; import co.rsk.bitcoinj.script.Script; import co.rsk.bitcoinj.script.ScriptOpCodes; import co.rsk.config.BridgeConstants; @@ -36,6 +35,7 @@ import java.util.Arrays; import java.util.Collections; import java.util.List; +import java.util.Optional; import java.util.stream.Collectors; import org.bouncycastle.util.encoders.Hex; import org.ethereum.config.blockchain.upgrades.ActivationConfig; @@ -697,6 +697,27 @@ void spendFromErpFed_after_RSKIP293_mainnet_using_standard_multisig() { )); } + @Test + void test_getMemberByBtcPublicKey_passing_existing_btcPublicKey(){ + BtcECKey existingMemberBtcPublicKey = standardKeys.get(0); + Optional foundMember = federation.getMemberByBtcPublicKey(existingMemberBtcPublicKey); + Assertions.assertTrue(foundMember.isPresent()); + Assertions.assertEquals(existingMemberBtcPublicKey, foundMember.get().getBtcPublicKey()); + } + + @Test + void test_getMemberByBtcPublicKey_passing_non_existing_btcPublicKey(){ + BtcECKey noExistingBtcPublicKey = new BtcECKey(); + Optional foundMember = federation.getMemberByBtcPublicKey(noExistingBtcPublicKey); + Assertions.assertFalse(foundMember.isPresent()); + } + + @Test + void test_getMemberByBtcPublicKey_passing_null_btcPublicKey(){ + Optional foundMember = federation.getMemberByBtcPublicKey(null); + Assertions.assertFalse(foundMember.isPresent()); + } + private void createErpFederation(BridgeConstants constants, boolean isRskip293Active) { when(activations.isActive(ConsensusRule.RSKIP293)).thenReturn(isRskip293Active); diff --git a/rskj-core/src/test/java/co/rsk/peg/P2shErpFederationTest.java b/rskj-core/src/test/java/co/rsk/peg/P2shErpFederationTest.java index 46908b100cd..b79c0db298a 100644 --- a/rskj-core/src/test/java/co/rsk/peg/P2shErpFederationTest.java +++ b/rskj-core/src/test/java/co/rsk/peg/P2shErpFederationTest.java @@ -23,6 +23,7 @@ import java.util.Arrays; import java.util.Collections; import java.util.List; +import java.util.Optional; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -454,6 +455,27 @@ void spendFromP2shErpFed( )); } + @Test + void test_getMemberByBtcPublicKey_passing_existing_btcPublicKey(){ + BtcECKey existingMemberBtcPublicKey = standardKeys.get(0); + Optional foundMember = federation.getMemberByBtcPublicKey(existingMemberBtcPublicKey); + Assertions.assertTrue(foundMember.isPresent()); + Assertions.assertEquals(existingMemberBtcPublicKey, foundMember.get().getBtcPublicKey()); + } + + @Test + void test_getMemberByBtcPublicKey_passing_non_existing_btcPublicKey(){ + BtcECKey noExistingBtcPublicKey = new BtcECKey(); + Optional foundMember = federation.getMemberByBtcPublicKey(noExistingBtcPublicKey); + Assertions.assertFalse(foundMember.isPresent()); + } + + @Test + void test_getMemberByBtcPublicKey_passing_null_btcPublicKey(){ + Optional foundMember = federation.getMemberByBtcPublicKey(null); + Assertions.assertFalse(foundMember.isPresent()); + } + private void validateP2shErpRedeemScript( Script erpRedeemScript, List defaultMultisigKeys, diff --git a/rskj-core/src/test/java/co/rsk/peg/StandardMultisigFederationTest.java b/rskj-core/src/test/java/co/rsk/peg/StandardMultisigFederationTest.java index fee7f0045e7..13f061338c1 100644 --- a/rskj-core/src/test/java/co/rsk/peg/StandardMultisigFederationTest.java +++ b/rskj-core/src/test/java/co/rsk/peg/StandardMultisigFederationTest.java @@ -320,4 +320,25 @@ void isMember(){ FederationMember invalidPubKeys = new FederationMember(invalidBtcKey, invalidRskKey, invalidRskKey); assertFalse(federation.isMember(invalidPubKeys)); } + + @Test + void test_getMemberByBtcPublicKey_passing_existing_btcPublicKey(){ + BtcECKey existingMemberBtcPublicKey = sortedPublicKeys.get(0); + Optional foundMember = federation.getMemberByBtcPublicKey(existingMemberBtcPublicKey); + Assertions.assertTrue(foundMember.isPresent()); + Assertions.assertEquals(existingMemberBtcPublicKey, foundMember.get().getBtcPublicKey()); + } + + @Test + void test_getMemberByBtcPublicKey_passing_non_existing_btcPublicKey(){ + BtcECKey noExistingBtcPublicKey = new BtcECKey(); + Optional foundMember = federation.getMemberByBtcPublicKey(noExistingBtcPublicKey); + Assertions.assertFalse(foundMember.isPresent()); + } + + @Test + void test_getMemberByBtcPublicKey_passing_null_btcPublicKey(){ + Optional foundMember = federation.getMemberByBtcPublicKey(null); + Assertions.assertFalse(foundMember.isPresent()); + } } From 5dc82e94fbe34a382ef420d95511b0b6a450866e Mon Sep 17 00:00:00 2001 From: nathanieliov Date: Tue, 30 Jan 2024 11:00:50 -0400 Subject: [PATCH 03/18] - Change findFirst to findAny on getMemberByBtcPublicKey method - Improved naming convention for the unit tests added --- rskj-core/src/main/java/co/rsk/peg/Federation.java | 2 +- .../src/test/java/co/rsk/peg/LegacyErpFederationTest.java | 6 +++--- .../src/test/java/co/rsk/peg/P2shErpFederationTest.java | 6 +++--- .../java/co/rsk/peg/StandardMultisigFederationTest.java | 6 +++--- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/rskj-core/src/main/java/co/rsk/peg/Federation.java b/rskj-core/src/main/java/co/rsk/peg/Federation.java index 0ad41d9cc50..f45579d18a8 100644 --- a/rskj-core/src/main/java/co/rsk/peg/Federation.java +++ b/rskj-core/src/main/java/co/rsk/peg/Federation.java @@ -65,7 +65,7 @@ public List getMembers() { } public Optional getMemberByBtcPublicKey(BtcECKey btcPublicKey) { - return members.stream().filter(federationMember -> federationMember.getBtcPublicKey().equals(btcPublicKey)).findFirst(); + return members.stream().filter(federationMember -> federationMember.getBtcPublicKey().equals(btcPublicKey)).findAny(); } public List getBtcPublicKeys() { diff --git a/rskj-core/src/test/java/co/rsk/peg/LegacyErpFederationTest.java b/rskj-core/src/test/java/co/rsk/peg/LegacyErpFederationTest.java index 2a6c929186f..f2e175a723e 100644 --- a/rskj-core/src/test/java/co/rsk/peg/LegacyErpFederationTest.java +++ b/rskj-core/src/test/java/co/rsk/peg/LegacyErpFederationTest.java @@ -698,7 +698,7 @@ void spendFromErpFed_after_RSKIP293_mainnet_using_standard_multisig() { } @Test - void test_getMemberByBtcPublicKey_passing_existing_btcPublicKey(){ + void getMemberByBtcPublicKey_passing_existing_btcPublicKey_should_return_found_member(){ BtcECKey existingMemberBtcPublicKey = standardKeys.get(0); Optional foundMember = federation.getMemberByBtcPublicKey(existingMemberBtcPublicKey); Assertions.assertTrue(foundMember.isPresent()); @@ -706,14 +706,14 @@ void test_getMemberByBtcPublicKey_passing_existing_btcPublicKey(){ } @Test - void test_getMemberByBtcPublicKey_passing_non_existing_btcPublicKey(){ + void getMemberByBtcPublicKey_passing_non_existing_btcPublicKey_should_return_empty(){ BtcECKey noExistingBtcPublicKey = new BtcECKey(); Optional foundMember = federation.getMemberByBtcPublicKey(noExistingBtcPublicKey); Assertions.assertFalse(foundMember.isPresent()); } @Test - void test_getMemberByBtcPublicKey_passing_null_btcPublicKey(){ + void getMemberByBtcPublicKey_passing_null_btcPublicKey_should_return_empty(){ Optional foundMember = federation.getMemberByBtcPublicKey(null); Assertions.assertFalse(foundMember.isPresent()); } diff --git a/rskj-core/src/test/java/co/rsk/peg/P2shErpFederationTest.java b/rskj-core/src/test/java/co/rsk/peg/P2shErpFederationTest.java index b79c0db298a..999a506f9a7 100644 --- a/rskj-core/src/test/java/co/rsk/peg/P2shErpFederationTest.java +++ b/rskj-core/src/test/java/co/rsk/peg/P2shErpFederationTest.java @@ -456,7 +456,7 @@ void spendFromP2shErpFed( } @Test - void test_getMemberByBtcPublicKey_passing_existing_btcPublicKey(){ + void getMemberByBtcPublicKey_passing_existing_btcPublicKey_should_return_found_member(){ BtcECKey existingMemberBtcPublicKey = standardKeys.get(0); Optional foundMember = federation.getMemberByBtcPublicKey(existingMemberBtcPublicKey); Assertions.assertTrue(foundMember.isPresent()); @@ -464,14 +464,14 @@ void test_getMemberByBtcPublicKey_passing_existing_btcPublicKey(){ } @Test - void test_getMemberByBtcPublicKey_passing_non_existing_btcPublicKey(){ + void getMemberByBtcPublicKey_passing_non_existing_btcPublicKey_should_return_empty(){ BtcECKey noExistingBtcPublicKey = new BtcECKey(); Optional foundMember = federation.getMemberByBtcPublicKey(noExistingBtcPublicKey); Assertions.assertFalse(foundMember.isPresent()); } @Test - void test_getMemberByBtcPublicKey_passing_null_btcPublicKey(){ + void getMemberByBtcPublicKey_passing_null_btcPublicKey_should_return_empty(){ Optional foundMember = federation.getMemberByBtcPublicKey(null); Assertions.assertFalse(foundMember.isPresent()); } diff --git a/rskj-core/src/test/java/co/rsk/peg/StandardMultisigFederationTest.java b/rskj-core/src/test/java/co/rsk/peg/StandardMultisigFederationTest.java index 13f061338c1..2a0fc0d1e43 100644 --- a/rskj-core/src/test/java/co/rsk/peg/StandardMultisigFederationTest.java +++ b/rskj-core/src/test/java/co/rsk/peg/StandardMultisigFederationTest.java @@ -322,7 +322,7 @@ void isMember(){ } @Test - void test_getMemberByBtcPublicKey_passing_existing_btcPublicKey(){ + void getMemberByBtcPublicKey_passing_existing_btcPublicKey_should_return_found_member(){ BtcECKey existingMemberBtcPublicKey = sortedPublicKeys.get(0); Optional foundMember = federation.getMemberByBtcPublicKey(existingMemberBtcPublicKey); Assertions.assertTrue(foundMember.isPresent()); @@ -330,14 +330,14 @@ void test_getMemberByBtcPublicKey_passing_existing_btcPublicKey(){ } @Test - void test_getMemberByBtcPublicKey_passing_non_existing_btcPublicKey(){ + void getMemberByBtcPublicKey_passing_non_existing_btcPublicKey_should_return_empty(){ BtcECKey noExistingBtcPublicKey = new BtcECKey(); Optional foundMember = federation.getMemberByBtcPublicKey(noExistingBtcPublicKey); Assertions.assertFalse(foundMember.isPresent()); } @Test - void test_getMemberByBtcPublicKey_passing_null_btcPublicKey(){ + void getMemberByBtcPublicKey_passing_null_btcPublicKey_should_return_empty(){ Optional foundMember = federation.getMemberByBtcPublicKey(null); Assertions.assertFalse(foundMember.isPresent()); } From 915ad7961a9c17bd7d75a6448d51ea3614ea0632 Mon Sep 17 00:00:00 2001 From: jeremy-then Date: Wed, 31 Jan 2024 12:56:50 -0400 Subject: [PATCH 04/18] Changes RemascFederationProvider.getFederatorAddress logic and adds tests --- .../src/main/java/co/rsk/remasc/Remasc.java | 17 +++- .../rsk/remasc/RemascFederationProvider.java | 34 +++----- .../remasc/RemascFederationProviderTest.java | 85 +++++++++++++++++-- .../remasc/RemascProcessMinerFeesTest.java | 19 ++++- .../rsk/remasc/RemascStorageProviderTest.java | 50 +++++++++-- 5 files changed, 165 insertions(+), 40 deletions(-) diff --git a/rskj-core/src/main/java/co/rsk/remasc/Remasc.java b/rskj-core/src/main/java/co/rsk/remasc/Remasc.java index db430403df6..8a7de72a61f 100644 --- a/rskj-core/src/main/java/co/rsk/remasc/Remasc.java +++ b/rskj-core/src/main/java/co/rsk/remasc/Remasc.java @@ -22,6 +22,8 @@ import co.rsk.core.Coin; import co.rsk.core.RskAddress; import co.rsk.core.bc.SelectionRule; +import co.rsk.peg.BridgeStorageProvider; +import co.rsk.peg.FederationSupport; import co.rsk.rpc.modules.trace.ProgramSubtrace; import org.ethereum.config.Constants; import org.ethereum.config.blockchain.upgrades.ActivationConfig; @@ -32,6 +34,7 @@ import org.ethereum.core.Transaction; import org.ethereum.db.BlockStore; import org.ethereum.vm.LogInfo; +import org.ethereum.vm.PrecompiledContracts; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -199,7 +202,19 @@ RskAddress getRskLabsAddress() { } private Coin payToFederation(Constants constants, boolean isRskip85Enabled, Block processingBlock, BlockHeader processingBlockHeader, Coin syntheticReward) { - RemascFederationProvider federationProvider = new RemascFederationProvider(activationConfig, constants.getBridgeConstants(), repository, processingBlock); + + ActivationConfig.ForBlock activations = activationConfig.forBlock(processingBlock.getNumber()); + + BridgeStorageProvider bridgeStorageProvider = new BridgeStorageProvider( + repository, + PrecompiledContracts.BRIDGE_ADDR, + constants.getBridgeConstants(), + activations + ); + + FederationSupport federationSupport = new FederationSupport(constants.getBridgeConstants(), bridgeStorageProvider, processingBlock, activations); + + RemascFederationProvider federationProvider = new RemascFederationProvider(activations, federationSupport); Coin federationReward = syntheticReward.divide(BigInteger.valueOf(remascConstants.getFederationDivisor())); Coin payToFederation = provider.getFederationBalance().add(federationReward); diff --git a/rskj-core/src/main/java/co/rsk/remasc/RemascFederationProvider.java b/rskj-core/src/main/java/co/rsk/remasc/RemascFederationProvider.java index ae5c8fcc37b..913e84daaf8 100644 --- a/rskj-core/src/main/java/co/rsk/remasc/RemascFederationProvider.java +++ b/rskj-core/src/main/java/co/rsk/remasc/RemascFederationProvider.java @@ -17,36 +17,26 @@ */ package co.rsk.remasc; -import co.rsk.config.BridgeConstants; import co.rsk.core.RskAddress; -import co.rsk.peg.BridgeStorageProvider; +import co.rsk.peg.FederationMember; import co.rsk.peg.FederationSupport; import org.ethereum.config.blockchain.upgrades.ActivationConfig; -import org.ethereum.core.Block; -import org.ethereum.core.Repository; +import org.ethereum.config.blockchain.upgrades.ConsensusRule; import org.ethereum.crypto.ECKey; -import org.ethereum.vm.PrecompiledContracts; /** * Created by ajlopez on 14/11/2017. */ public class RemascFederationProvider { + private final FederationSupport federationSupport; + private final ActivationConfig.ForBlock activations; public RemascFederationProvider( - ActivationConfig activationConfig, - BridgeConstants bridgeConstants, - Repository repository, - Block processingBlock) { - - ActivationConfig.ForBlock activations = activationConfig.forBlock(processingBlock.getNumber()); - BridgeStorageProvider bridgeStorageProvider = new BridgeStorageProvider( - repository, - PrecompiledContracts.BRIDGE_ADDR, - bridgeConstants, - activations - ); - this.federationSupport = new FederationSupport(bridgeConstants, bridgeStorageProvider, processingBlock, activations); + ActivationConfig.ForBlock activations, + FederationSupport federationSupport) { + this.federationSupport = federationSupport; + this.activations = activations; } public int getFederationSize() { @@ -54,7 +44,11 @@ public int getFederationSize() { } public RskAddress getFederatorAddress(int n) { - byte[] publicKey = this.federationSupport.getFederatorBtcPublicKey(n); - return new RskAddress(ECKey.fromPublicOnly(publicKey).getAddress()); + if(!activations.isActive(ConsensusRule.RSKIP415)) { + byte[] btcPublicKey = this.federationSupport.getFederatorBtcPublicKey(n); + return new RskAddress(ECKey.fromPublicOnly(btcPublicKey).getAddress()); + } + byte[] rskPublicKey = this.federationSupport.getFederatorPublicKeyOfType(n, FederationMember.KeyType.RSK); + return new RskAddress(ECKey.fromPublicOnly(rskPublicKey).getAddress()); } } diff --git a/rskj-core/src/test/java/co/rsk/remasc/RemascFederationProviderTest.java b/rskj-core/src/test/java/co/rsk/remasc/RemascFederationProviderTest.java index b2d318d71f5..84cd250d782 100644 --- a/rskj-core/src/test/java/co/rsk/remasc/RemascFederationProviderTest.java +++ b/rskj-core/src/test/java/co/rsk/remasc/RemascFederationProviderTest.java @@ -2,17 +2,30 @@ import co.rsk.blockchain.utils.BlockGenerator; import co.rsk.config.BridgeRegTestConstants; +import co.rsk.core.RskAddress; +import co.rsk.peg.*; import co.rsk.test.builders.BlockChainBuilder; +import co.rsk.util.HexUtils; +import org.ethereum.config.blockchain.upgrades.ActivationConfig; import org.ethereum.config.blockchain.upgrades.ActivationConfigsForTest; +import org.ethereum.config.blockchain.upgrades.ConsensusRule; import org.ethereum.core.Blockchain; import org.ethereum.core.Genesis; +import org.ethereum.crypto.ECKey; +import org.ethereum.vm.PrecompiledContracts; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; +import static org.mockito.Mockito.*; + /** * Created by ajlopez on 14/11/2017. */ class RemascFederationProviderTest { + + private static final String BTC_PUBLIC_KEY = "0x03a5e32151f974c35c5d08af36a8d6af0593248e8e4adca7ed0148192eb2f5d1bf"; + private static final String RSK_PUBLIC_KEY = "0x02f3b5fb3121932db9450121b0a37f3f051dc8b3fd5f1ea5e7cf726947d8f69c28"; + @Test void getDefaultFederationSize() { RemascFederationProvider provider = getRemascFederationProvider(); @@ -20,25 +33,79 @@ void getDefaultFederationSize() { } @Test - void getFederatorAddress() { - RemascFederationProvider provider = getRemascFederationProvider(); + void getFederatorAddress_preRSKIP415_returnsRskAddressDerivedFromBtcPublicKey() { + + int federatorIndex = 0; + + ActivationConfig.ForBlock activations = mock(ActivationConfig.ForBlock.class); + when(activations.isActive(ConsensusRule.RSKIP415)).thenReturn(false); + + FederationSupport federationSupport = mock(FederationSupport.class); + + byte[] btcPublicKey = HexUtils.strHexOrStrNumberToByteArray(BTC_PUBLIC_KEY); - byte[] address = provider.getFederatorAddress(0).getBytes(); + when(federationSupport.getFederatorBtcPublicKey(federatorIndex)) + .thenReturn(btcPublicKey); + + RemascFederationProvider provider = new RemascFederationProvider(activations, federationSupport); + + RskAddress actualRskAddress = provider.getFederatorAddress(federatorIndex); + RskAddress expectedRskAddress = new RskAddress(ECKey.fromPublicOnly(btcPublicKey).getAddress()); + + Assertions.assertEquals(expectedRskAddress, actualRskAddress); + verify(federationSupport, never()).getFederatorPublicKeyOfType(federatorIndex, FederationMember.KeyType.RSK); + verify(federationSupport, times(1)).getFederatorBtcPublicKey(federatorIndex); + + } + + @Test + void getFederatorAddress_postRSKIP415_returnsRskAddressDerivedFromRskPublicKey() { + + int federatorIndex = 0; + + ActivationConfig.ForBlock activations = mock(ActivationConfig.ForBlock.class); + when(activations.isActive(ConsensusRule.RSKIP415)).thenReturn(true); + + FederationSupport federationSupport = mock(FederationSupport.class); + + byte[] rskPublicKey = HexUtils.strHexOrStrNumberToByteArray(RSK_PUBLIC_KEY); + + when(federationSupport.getFederatorPublicKeyOfType(federatorIndex, FederationMember.KeyType.RSK)) + .thenReturn(rskPublicKey); + + RemascFederationProvider provider = new RemascFederationProvider(activations, federationSupport); + + RskAddress actualRskAddress = provider.getFederatorAddress(federatorIndex); + RskAddress expectedRskAddress = new RskAddress(ECKey.fromPublicOnly(rskPublicKey).getAddress()); + + Assertions.assertEquals(expectedRskAddress, actualRskAddress); + verify(federationSupport, never()).getFederatorBtcPublicKey(federatorIndex); + verify(federationSupport, times(1)).getFederatorPublicKeyOfType(federatorIndex, FederationMember.KeyType.RSK); - Assertions.assertNotNull(address); - Assertions.assertEquals(20, address.length); } private static RemascFederationProvider getRemascFederationProvider() { + Genesis genesisBlock = new BlockGenerator().getGenesisBlock(); BlockChainBuilder builder = new BlockChainBuilder().setTesting(true).setGenesis(genesisBlock); Blockchain blockchain = builder.build(); - return new RemascFederationProvider( - ActivationConfigsForTest.all(), - BridgeRegTestConstants.getInstance(), + ActivationConfig.ForBlock activations = ActivationConfigsForTest.all().forBlock(blockchain.getBestBlock().getNumber()); + + BridgeRegTestConstants bridgeRegTestConstants = BridgeRegTestConstants.getInstance(); + + BridgeStorageProvider bridgeStorageProvider = new BridgeStorageProvider( builder.getRepository(), - blockchain.getBestBlock() + PrecompiledContracts.BRIDGE_ADDR, + bridgeRegTestConstants, + activations + ); + + FederationSupport federationSupport = new FederationSupport(bridgeRegTestConstants, bridgeStorageProvider, blockchain.getBestBlock(), activations); + + return new RemascFederationProvider( + activations, + federationSupport ); } } diff --git a/rskj-core/src/test/java/co/rsk/remasc/RemascProcessMinerFeesTest.java b/rskj-core/src/test/java/co/rsk/remasc/RemascProcessMinerFeesTest.java index 4dc1ebf7f17..572aff76f27 100644 --- a/rskj-core/src/test/java/co/rsk/remasc/RemascProcessMinerFeesTest.java +++ b/rskj-core/src/test/java/co/rsk/remasc/RemascProcessMinerFeesTest.java @@ -32,12 +32,11 @@ import co.rsk.db.RepositorySnapshot; import co.rsk.db.StateRootHandler; import co.rsk.db.StateRootsStoreImpl; -import co.rsk.peg.BridgeSupportFactory; -import co.rsk.peg.PegTestUtils; -import co.rsk.peg.RepositoryBtcBlockStoreWithCache; +import co.rsk.peg.*; import co.rsk.test.builders.BlockChainBuilder; import org.bouncycastle.util.encoders.Hex; import org.ethereum.TestUtils; +import org.ethereum.config.blockchain.upgrades.ActivationConfig; import org.ethereum.config.blockchain.upgrades.ActivationConfigsForTest; import org.ethereum.config.blockchain.upgrades.ConsensusRule; import org.ethereum.core.*; @@ -966,7 +965,19 @@ private HashMap getAccountsWithExpectedBalance(List otherAcc } private void validateFederatorsBalanceIsCorrect(RepositorySnapshot repository, long federationReward, Block executionBlock) { - RemascFederationProvider provider = new RemascFederationProvider(config.getActivationConfig(), config.getNetworkConstants().getBridgeConstants(), repository.startTracking(), executionBlock); + + ActivationConfig.ForBlock activations = config.getActivationConfig().forBlock(executionBlock.getNumber()); + + BridgeStorageProvider bridgeStorageProvider = new BridgeStorageProvider( + repository.startTracking(), + PrecompiledContracts.BRIDGE_ADDR, + config.getNetworkConstants().getBridgeConstants(), + activations + ); + + FederationSupport federationSupport = new FederationSupport(config.getNetworkConstants().getBridgeConstants(), bridgeStorageProvider, executionBlock, activations); + + RemascFederationProvider provider = new RemascFederationProvider(config.getActivationConfig().forBlock(executionBlock.getNumber()), federationSupport); int nfederators = provider.getFederationSize(); diff --git a/rskj-core/src/test/java/co/rsk/remasc/RemascStorageProviderTest.java b/rskj-core/src/test/java/co/rsk/remasc/RemascStorageProviderTest.java index 14ab83b2a37..c29b63d5f8a 100644 --- a/rskj-core/src/test/java/co/rsk/remasc/RemascStorageProviderTest.java +++ b/rskj-core/src/test/java/co/rsk/remasc/RemascStorageProviderTest.java @@ -29,13 +29,12 @@ import co.rsk.db.MutableTrieImpl; import co.rsk.db.RepositoryLocator; import co.rsk.db.RepositorySnapshot; -import co.rsk.peg.BridgeSupportFactory; -import co.rsk.peg.PegTestUtils; -import co.rsk.peg.RepositoryBtcBlockStoreWithCache; +import co.rsk.peg.*; import co.rsk.test.builders.BlockChainBuilder; import co.rsk.trie.Trie; import org.ethereum.TestUtils; import org.ethereum.config.Constants; +import org.ethereum.config.blockchain.upgrades.ActivationConfig; import org.ethereum.config.blockchain.upgrades.ActivationConfigsForTest; import org.ethereum.config.blockchain.upgrades.ConsensusRule; import org.ethereum.core.*; @@ -293,7 +292,20 @@ void alwaysPaysFedBeforeRFS() throws IOException { Blockchain blockchain = testRunner.getBlockChain(); RepositoryLocator repositoryLocator = builder.getRepositoryLocator(); RepositorySnapshot repository = repositoryLocator.snapshotAt(blockchain.getBestBlock().getHeader()); - RemascFederationProvider federationProvider = new RemascFederationProvider(config.getActivationConfig(), config.getNetworkConstants().getBridgeConstants(), repository.startTracking(), testRunner.getBlockChain().getBestBlock()); + Block executionBlock = testRunner.getBlockChain().getBestBlock(); + + ActivationConfig.ForBlock activations = config.getActivationConfig().forBlock(executionBlock.getNumber()); + + BridgeStorageProvider bridgeStorageProvider = new BridgeStorageProvider( + repository.startTracking(), + PrecompiledContracts.BRIDGE_ADDR, + config.getNetworkConstants().getBridgeConstants(), + activations + ); + + FederationSupport federationSupport = new FederationSupport(config.getNetworkConstants().getBridgeConstants(), bridgeStorageProvider, executionBlock, activations); + + RemascFederationProvider federationProvider = new RemascFederationProvider(config.getActivationConfig().forBlock(executionBlock.getNumber()), federationSupport); assertEquals(Coin.valueOf(0), this.getRemascStorageProvider(repository).getFederationBalance()); long federatorBalance = (168 / federationProvider.getFederationSize()) * 2; assertEquals(Coin.valueOf(federatorBalance), RemascTestRunner.getAccountBalance(repository, federationProvider.getFederatorAddress(0))); @@ -320,7 +332,20 @@ void doesntPayFedBelowMinimumRewardAfterRFS() throws IOException { Blockchain blockchain = testRunner.getBlockChain(); RepositoryLocator repositoryLocator = builder.getRepositoryLocator(); RepositorySnapshot repository = repositoryLocator.snapshotAt(blockchain.getBestBlock().getHeader()); - RemascFederationProvider federationProvider = new RemascFederationProvider(config.getActivationConfig(), config.getNetworkConstants().getBridgeConstants(), repository.startTracking(), testRunner.getBlockChain().getBestBlock()); + Block executionBlock = testRunner.getBlockChain().getBestBlock(); + + ActivationConfig.ForBlock activations = config.getActivationConfig().forBlock(executionBlock.getNumber()); + + BridgeStorageProvider bridgeStorageProvider = new BridgeStorageProvider( + repository.startTracking(), + PrecompiledContracts.BRIDGE_ADDR, + config.getNetworkConstants().getBridgeConstants(), + activations + ); + + FederationSupport federationSupport = new FederationSupport(config.getNetworkConstants().getBridgeConstants(), bridgeStorageProvider, executionBlock, activations); + + RemascFederationProvider federationProvider = new RemascFederationProvider(config.getActivationConfig().forBlock(executionBlock.getNumber()), federationSupport); assertEquals(Coin.valueOf(336), this.getRemascStorageProvider(repository).getFederationBalance()); assertEquals(null, RemascTestRunner.getAccountBalance(repository, federationProvider.getFederatorAddress(0))); } @@ -370,7 +395,20 @@ void paysFedWhenHigherThanMinimumRewardAfterRFS() throws IOException { Blockchain blockchain = testRunner.getBlockChain(); RepositoryLocator repositoryLocator = builder.getRepositoryLocator(); RepositorySnapshot repository = repositoryLocator.snapshotAt(blockchain.getBestBlock().getHeader()); - RemascFederationProvider federationProvider = new RemascFederationProvider(config.getActivationConfig(), config.getNetworkConstants().getBridgeConstants(), repository.startTracking(), testRunner.getBlockChain().getBestBlock()); + Block executionBlock = testRunner.getBlockChain().getBestBlock(); + + ActivationConfig.ForBlock activations = config.getActivationConfig().forBlock(executionBlock.getNumber()); + + BridgeStorageProvider bridgeStorageProvider = new BridgeStorageProvider( + repository.startTracking(), + PrecompiledContracts.BRIDGE_ADDR, + config.getNetworkConstants().getBridgeConstants(), + activations + ); + + FederationSupport federationSupport = new FederationSupport(config.getNetworkConstants().getBridgeConstants(), bridgeStorageProvider, executionBlock, activations); + + RemascFederationProvider federationProvider = new RemascFederationProvider(config.getActivationConfig().forBlock(executionBlock.getNumber()), federationSupport); long federatorBalance = (1680 / federationProvider.getFederationSize()) * 2; assertEquals(Coin.valueOf(0), this.getRemascStorageProvider(repository).getFederationBalance()); assertEquals(Coin.valueOf(federatorBalance), RemascTestRunner.getAccountBalance(repository, federationProvider.getFederatorAddress(0))); From 925ef8ec947a4e66d7b012d60aa793cedc96cd9d Mon Sep 17 00:00:00 2001 From: jeremy-then Date: Wed, 31 Jan 2024 17:59:03 -0400 Subject: [PATCH 05/18] Refactors RemascFederationProvider::getFederatorAddress and creates more utility functions for clean and readable code --- .../src/main/java/co/rsk/remasc/Remasc.java | 13 +++++++----- .../rsk/remasc/RemascFederationProvider.java | 19 ++++++++++++++--- .../remasc/RemascFederationProviderTest.java | 21 +++++++++---------- 3 files changed, 34 insertions(+), 19 deletions(-) diff --git a/rskj-core/src/main/java/co/rsk/remasc/Remasc.java b/rskj-core/src/main/java/co/rsk/remasc/Remasc.java index 8a7de72a61f..4ea53fbb837 100644 --- a/rskj-core/src/main/java/co/rsk/remasc/Remasc.java +++ b/rskj-core/src/main/java/co/rsk/remasc/Remasc.java @@ -18,6 +18,7 @@ package co.rsk.remasc; +import co.rsk.config.BridgeConstants; import co.rsk.config.RemascConfig; import co.rsk.core.Coin; import co.rsk.core.RskAddress; @@ -179,7 +180,7 @@ void processMinersFees() { Coin payToRskLabs = syntheticReward.divide(BigInteger.valueOf(remascConstants.getRskLabsDivisor())); feesPayer.payMiningFees(processingBlockHeader.getHash().getBytes(), payToRskLabs, rskLabsAddress, logs); syntheticReward = syntheticReward.subtract(payToRskLabs); - Coin payToFederation = payToFederation(constants, isRskip85Enabled, processingBlock, processingBlockHeader, syntheticReward); + Coin payToFederation = payToFederation(constants, processingBlock, processingBlockHeader, syntheticReward); syntheticReward = syntheticReward.subtract(payToFederation); if (!siblings.isEmpty()) { @@ -201,18 +202,20 @@ RskAddress getRskLabsAddress() { return isRskip218Enabled ? remascConstants.getRskLabsAddressRskip218() : remascConstants.getRskLabsAddress(); } - private Coin payToFederation(Constants constants, boolean isRskip85Enabled, Block processingBlock, BlockHeader processingBlockHeader, Coin syntheticReward) { + private Coin payToFederation(Constants constants, Block processingBlock, BlockHeader processingBlockHeader, Coin syntheticReward) { ActivationConfig.ForBlock activations = activationConfig.forBlock(processingBlock.getNumber()); + BridgeConstants bridgeConstants = constants.getBridgeConstants(); + BridgeStorageProvider bridgeStorageProvider = new BridgeStorageProvider( repository, PrecompiledContracts.BRIDGE_ADDR, - constants.getBridgeConstants(), + bridgeConstants, activations ); - FederationSupport federationSupport = new FederationSupport(constants.getBridgeConstants(), bridgeStorageProvider, processingBlock, activations); + FederationSupport federationSupport = new FederationSupport(bridgeConstants, bridgeStorageProvider, processingBlock, activations); RemascFederationProvider federationProvider = new RemascFederationProvider(activations, federationSupport); Coin federationReward = syntheticReward.divide(BigInteger.valueOf(remascConstants.getFederationDivisor())); @@ -224,7 +227,7 @@ private Coin payToFederation(Constants constants, boolean isRskip85Enabled, Bloc Coin payToFederator = payAndRemainderToFederator[0]; Coin restToLastFederator = payAndRemainderToFederator[1]; - if (isRskip85Enabled) { + if (activations.isActive(ConsensusRule.RSKIP85)) { BigInteger minimumFederatorPayableGas = constants.getFederatorMinimumPayableGas(); Coin minPayableFederatorFees = executionBlock.getMinimumGasPrice().multiply(minimumFederatorPayableGas); if (payToFederator.compareTo(minPayableFederatorFees) < 0) { diff --git a/rskj-core/src/main/java/co/rsk/remasc/RemascFederationProvider.java b/rskj-core/src/main/java/co/rsk/remasc/RemascFederationProvider.java index 913e84daaf8..2d76cc3198a 100644 --- a/rskj-core/src/main/java/co/rsk/remasc/RemascFederationProvider.java +++ b/rskj-core/src/main/java/co/rsk/remasc/RemascFederationProvider.java @@ -45,10 +45,23 @@ public int getFederationSize() { public RskAddress getFederatorAddress(int n) { if(!activations.isActive(ConsensusRule.RSKIP415)) { - byte[] btcPublicKey = this.federationSupport.getFederatorBtcPublicKey(n); - return new RskAddress(ECKey.fromPublicOnly(btcPublicKey).getAddress()); + return getRskAddressFromBtcKey(n); } + return getRskAddressFromRskKey(n); + } + + private RskAddress getRskAddressFromBtcKey(int n) { + byte[] btcPublicKey = this.federationSupport.getFederatorBtcPublicKey(n); + return getRskAddressFromKey(btcPublicKey); + } + + private RskAddress getRskAddressFromRskKey(int n) { byte[] rskPublicKey = this.federationSupport.getFederatorPublicKeyOfType(n, FederationMember.KeyType.RSK); - return new RskAddress(ECKey.fromPublicOnly(rskPublicKey).getAddress()); + return getRskAddressFromKey(rskPublicKey); } + + private RskAddress getRskAddressFromKey(byte[] publicKey) { + return new RskAddress(ECKey.fromPublicOnly(publicKey).getAddress()); + } + } diff --git a/rskj-core/src/test/java/co/rsk/remasc/RemascFederationProviderTest.java b/rskj-core/src/test/java/co/rsk/remasc/RemascFederationProviderTest.java index 84cd250d782..c91db4385cb 100644 --- a/rskj-core/src/test/java/co/rsk/remasc/RemascFederationProviderTest.java +++ b/rskj-core/src/test/java/co/rsk/remasc/RemascFederationProviderTest.java @@ -1,7 +1,7 @@ package co.rsk.remasc; import co.rsk.blockchain.utils.BlockGenerator; -import co.rsk.config.BridgeRegTestConstants; +import co.rsk.config.BridgeMainNetConstants; import co.rsk.core.RskAddress; import co.rsk.peg.*; import co.rsk.test.builders.BlockChainBuilder; @@ -11,7 +11,6 @@ import org.ethereum.config.blockchain.upgrades.ConsensusRule; import org.ethereum.core.Blockchain; import org.ethereum.core.Genesis; -import org.ethereum.crypto.ECKey; import org.ethereum.vm.PrecompiledContracts; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; @@ -24,12 +23,14 @@ class RemascFederationProviderTest { private static final String BTC_PUBLIC_KEY = "0x03a5e32151f974c35c5d08af36a8d6af0593248e8e4adca7ed0148192eb2f5d1bf"; + private static final String RSK_ADDRESS_FROM_BTC_PUBLIC_KEY = "131c7c821b0e4a1ab570feed952d9304e03fddd1"; private static final String RSK_PUBLIC_KEY = "0x02f3b5fb3121932db9450121b0a37f3f051dc8b3fd5f1ea5e7cf726947d8f69c28"; + private static final String RSK_ADDRESS_FROM_RSK_PUBLIC_KEY = "54a948b3d76ce84e5c7b4b3cd01f2af1f18d41e0"; @Test void getDefaultFederationSize() { RemascFederationProvider provider = getRemascFederationProvider(); - Assertions.assertEquals(3, provider.getFederationSize()); + Assertions.assertEquals(15, provider.getFederationSize()); } @Test @@ -50,9 +51,8 @@ void getFederatorAddress_preRSKIP415_returnsRskAddressDerivedFromBtcPublicKey() RemascFederationProvider provider = new RemascFederationProvider(activations, federationSupport); RskAddress actualRskAddress = provider.getFederatorAddress(federatorIndex); - RskAddress expectedRskAddress = new RskAddress(ECKey.fromPublicOnly(btcPublicKey).getAddress()); - Assertions.assertEquals(expectedRskAddress, actualRskAddress); + Assertions.assertEquals(RSK_ADDRESS_FROM_BTC_PUBLIC_KEY, actualRskAddress.toHexString()); verify(federationSupport, never()).getFederatorPublicKeyOfType(federatorIndex, FederationMember.KeyType.RSK); verify(federationSupport, times(1)).getFederatorBtcPublicKey(federatorIndex); @@ -76,11 +76,10 @@ void getFederatorAddress_postRSKIP415_returnsRskAddressDerivedFromRskPublicKey() RemascFederationProvider provider = new RemascFederationProvider(activations, federationSupport); RskAddress actualRskAddress = provider.getFederatorAddress(federatorIndex); - RskAddress expectedRskAddress = new RskAddress(ECKey.fromPublicOnly(rskPublicKey).getAddress()); - Assertions.assertEquals(expectedRskAddress, actualRskAddress); - verify(federationSupport, never()).getFederatorBtcPublicKey(federatorIndex); + Assertions.assertEquals(RSK_ADDRESS_FROM_RSK_PUBLIC_KEY, actualRskAddress.toHexString()); verify(federationSupport, times(1)).getFederatorPublicKeyOfType(federatorIndex, FederationMember.KeyType.RSK); + verify(federationSupport, never()).getFederatorBtcPublicKey(federatorIndex); } @@ -92,16 +91,16 @@ private static RemascFederationProvider getRemascFederationProvider() { ActivationConfig.ForBlock activations = ActivationConfigsForTest.all().forBlock(blockchain.getBestBlock().getNumber()); - BridgeRegTestConstants bridgeRegTestConstants = BridgeRegTestConstants.getInstance(); + BridgeMainNetConstants bridgeMainNetConstants = BridgeMainNetConstants.getInstance(); BridgeStorageProvider bridgeStorageProvider = new BridgeStorageProvider( builder.getRepository(), PrecompiledContracts.BRIDGE_ADDR, - bridgeRegTestConstants, + bridgeMainNetConstants, activations ); - FederationSupport federationSupport = new FederationSupport(bridgeRegTestConstants, bridgeStorageProvider, blockchain.getBestBlock(), activations); + FederationSupport federationSupport = new FederationSupport(bridgeMainNetConstants, bridgeStorageProvider, blockchain.getBestBlock(), activations); return new RemascFederationProvider( activations, From 88ac4e46cd09d42a98ea2e846a127ea7271c79b6 Mon Sep 17 00:00:00 2001 From: nathanieliov Date: Tue, 30 Jan 2024 17:39:41 -0400 Subject: [PATCH 06/18] - Refactor logAddSignature method - Add and update existing tests for logAddSignature method --- .../main/java/co/rsk/peg/BridgeSupport.java | 12 ++-- .../co/rsk/peg/utils/BridgeEventLogger.java | 3 +- .../rsk/peg/utils/BridgeEventLoggerImpl.java | 18 +++-- .../peg/utils/BrigeEventLoggerLegacyImpl.java | 5 +- .../peg/BridgeSupportAddSignatureTest.java | 3 +- .../peg/utils/BridgeEventLoggerImplTest.java | 71 ++++++++++++++++--- .../BridgeEventLoggerLegacyImplTest.java | 6 +- .../upgrades/ActivationConfigsForTest.java | 3 +- 8 files changed, 97 insertions(+), 24 deletions(-) diff --git a/rskj-core/src/main/java/co/rsk/peg/BridgeSupport.java b/rskj-core/src/main/java/co/rsk/peg/BridgeSupport.java index 7404569f792..99b9367a330 100644 --- a/rskj-core/src/main/java/co/rsk/peg/BridgeSupport.java +++ b/rskj-core/src/main/java/co/rsk/peg/BridgeSupport.java @@ -1435,6 +1435,8 @@ public void addSignature(BtcECKey federatorPublicKey, List signatures, b } Federation federation = optionalFederation.get(); + FederationMember signingFederationMember = federation.getMemberByBtcPublicKey(federatorPublicKey).get(); + BtcTransaction btcTx = provider.getPegoutsWaitingForSignatures().get(new Keccak256(rskTxHash)); if (btcTx == null) { logger.warn( @@ -1453,10 +1455,10 @@ public void addSignature(BtcECKey federatorPublicKey, List signatures, b } if (!activations.isActive(ConsensusRule.RSKIP326)) { - eventLogger.logAddSignature(federatorPublicKey, btcTx, rskTxHash); + eventLogger.logAddSignature(signingFederationMember, btcTx, rskTxHash); } - processSigning(federatorPublicKey, signatures, rskTxHash, btcTx, federation); + processSigning(signingFederationMember, signatures, rskTxHash, btcTx, federation); } private Optional getFederationFromPublicKey(BtcECKey federatorPublicKey) { @@ -1474,11 +1476,13 @@ private Optional getFederationFromPublicKey(BtcECKey federatorPublic } private void processSigning( - BtcECKey federatorPublicKey, + FederationMember federatorMember, List signatures, byte[] rskTxHash, BtcTransaction btcTx, Federation federation) throws IOException { + + BtcECKey federatorPublicKey = federatorMember.getBtcPublicKey(); // Build input hashes for signatures int numInputs = btcTx.getInputs().size(); @@ -1568,7 +1572,7 @@ private void processSigning( } if(signed && activations.isActive(ConsensusRule.RSKIP326)) { - eventLogger.logAddSignature(federatorPublicKey, btcTx, rskTxHash); + eventLogger.logAddSignature(federatorMember, btcTx, rskTxHash); } if (BridgeUtils.hasEnoughSignatures(btcContext, btcTx)) { diff --git a/rskj-core/src/main/java/co/rsk/peg/utils/BridgeEventLogger.java b/rskj-core/src/main/java/co/rsk/peg/utils/BridgeEventLogger.java index 62bc7f7121b..7152c778ff8 100644 --- a/rskj-core/src/main/java/co/rsk/peg/utils/BridgeEventLogger.java +++ b/rskj-core/src/main/java/co/rsk/peg/utils/BridgeEventLogger.java @@ -22,6 +22,7 @@ import co.rsk.core.RskAddress; import co.rsk.crypto.Keccak256; import co.rsk.peg.Federation; +import co.rsk.peg.FederationMember; import co.rsk.peg.pegin.RejectedPeginReason; import org.ethereum.core.Block; import org.ethereum.core.Transaction; @@ -36,7 +37,7 @@ public interface BridgeEventLogger { void logUpdateCollections(Transaction rskTx); - void logAddSignature(BtcECKey federatorPublicKey, BtcTransaction btcTx, byte[] rskTxHash); + void logAddSignature(FederationMember federationMember, BtcTransaction btcTx, byte[] rskTxHash); void logReleaseBtc(BtcTransaction btcTx, byte[] rskTxHash); diff --git a/rskj-core/src/main/java/co/rsk/peg/utils/BridgeEventLoggerImpl.java b/rskj-core/src/main/java/co/rsk/peg/utils/BridgeEventLoggerImpl.java index 9f8c328f975..3628b361b60 100644 --- a/rskj-core/src/main/java/co/rsk/peg/utils/BridgeEventLoggerImpl.java +++ b/rskj-core/src/main/java/co/rsk/peg/utils/BridgeEventLoggerImpl.java @@ -23,6 +23,7 @@ import co.rsk.crypto.Keccak256; import co.rsk.peg.BridgeEvents; import co.rsk.peg.Federation; +import co.rsk.peg.FederationMember; import co.rsk.peg.pegin.RejectedPeginReason; import org.ethereum.config.blockchain.upgrades.ActivationConfig; import org.ethereum.config.blockchain.upgrades.ConsensusRule; @@ -75,10 +76,19 @@ public void logUpdateCollections(Transaction rskTx) { } @Override - public void logAddSignature(BtcECKey federatorPublicKey, BtcTransaction btcTx, byte[] rskTxHash) { - ECKey key = ECKey.fromPublicOnly(federatorPublicKey.getPubKey()); - String federatorRskAddress = ByteUtil.toHexString(key.getAddress()); - logAddSignatureInSolidityFormat(rskTxHash, federatorRskAddress, federatorPublicKey); + public void logAddSignature(FederationMember federationMember, BtcTransaction btcTx, byte[] rskTxHash) { + if (shouldUseRskPublicKey()){ + String federatorRskAddress = ByteUtil.toHexString(federationMember.getRskPublicKey().getAddress()); + logAddSignatureInSolidityFormat(rskTxHash, federatorRskAddress, federationMember.getBtcPublicKey()); + } else { + ECKey ecKeyFromBtcPublicKey = ECKey.fromPublicOnly(federationMember.getBtcPublicKey().getPubKey()); + String federatorRskAddressFromBtcPublicKey = ByteUtil.toHexString(ecKeyFromBtcPublicKey.getAddress()); + logAddSignatureInSolidityFormat(rskTxHash, federatorRskAddressFromBtcPublicKey, federationMember.getBtcPublicKey()); + } + } + + private boolean shouldUseRskPublicKey() { + return activations.isActive(ConsensusRule.RSKIP415); } private void logAddSignatureInSolidityFormat(byte[] rskTxHash, String federatorRskAddress, BtcECKey federatorPublicKey) { diff --git a/rskj-core/src/main/java/co/rsk/peg/utils/BrigeEventLoggerLegacyImpl.java b/rskj-core/src/main/java/co/rsk/peg/utils/BrigeEventLoggerLegacyImpl.java index 81d521ea1cb..802550f8716 100644 --- a/rskj-core/src/main/java/co/rsk/peg/utils/BrigeEventLoggerLegacyImpl.java +++ b/rskj-core/src/main/java/co/rsk/peg/utils/BrigeEventLoggerLegacyImpl.java @@ -23,6 +23,7 @@ import co.rsk.peg.Bridge; import co.rsk.peg.DeprecatedMethodCallException; import co.rsk.peg.Federation; +import co.rsk.peg.FederationMember; import org.ethereum.config.blockchain.upgrades.ActivationConfig; import org.ethereum.config.blockchain.upgrades.ConsensusRule; import org.ethereum.core.Block; @@ -77,7 +78,7 @@ public void logUpdateCollections(Transaction rskTx) { } @Override - public void logAddSignature(BtcECKey federatorPublicKey, BtcTransaction btcTx, byte[] rskTxHash) { + public void logAddSignature(FederationMember federationMember, BtcTransaction btcTx, byte[] rskTxHash) { if (activations.isActive(ConsensusRule.RSKIP146)) { throw new DeprecatedMethodCallException( "Calling BrigeEventLoggerLegacyImpl.logAddSignature method after RSKIP146 activation" @@ -85,7 +86,7 @@ public void logAddSignature(BtcECKey federatorPublicKey, BtcTransaction btcTx, b } List topics = Collections.singletonList(Bridge.ADD_SIGNATURE_TOPIC); byte[] data = RLP.encodeList(RLP.encodeString(btcTx.getHashAsString()), - RLP.encodeElement(federatorPublicKey.getPubKeyHash()), + RLP.encodeElement(federationMember.getBtcPublicKey().getPubKeyHash()), RLP.encodeElement(rskTxHash)); this.logs.add(new LogInfo(BRIDGE_CONTRACT_ADDRESS, topics, data)); diff --git a/rskj-core/src/test/java/co/rsk/peg/BridgeSupportAddSignatureTest.java b/rskj-core/src/test/java/co/rsk/peg/BridgeSupportAddSignatureTest.java index 5f4744eef9e..941aed257b3 100644 --- a/rskj-core/src/test/java/co/rsk/peg/BridgeSupportAddSignatureTest.java +++ b/rskj-core/src/test/java/co/rsk/peg/BridgeSupportAddSignatureTest.java @@ -377,12 +377,13 @@ private void test_addSignature_EventEmitted(boolean rskip326Active, boolean useV } BtcECKey federatorPubKey = BridgeRegTestConstants.REGTEST_FEDERATION_PUBLIC_KEYS.get(indexOfKeyToSignWith); + FederationMember federationMember = FederationTestUtils.getFederationMemberWithKey(federatorPubKey); bridgeSupport.addSignature(federatorPubKey, derEncodedSigs, rskTxHash.getBytes()); if(shouldSignTwice) { bridgeSupport.addSignature(federatorPubKey, derEncodedSigs, rskTxHash.getBytes()); } - verify(eventLogger, times(wantedNumberOfInvocations)).logAddSignature(federatorPubKey, btcTx, rskTxHash.getBytes()); + verify(eventLogger, times(wantedNumberOfInvocations)).logAddSignature(federationMember, btcTx, rskTxHash.getBytes()); } @Test diff --git a/rskj-core/src/test/java/co/rsk/peg/utils/BridgeEventLoggerImplTest.java b/rskj-core/src/test/java/co/rsk/peg/utils/BridgeEventLoggerImplTest.java index 369dc1898bd..863524a0e81 100644 --- a/rskj-core/src/test/java/co/rsk/peg/utils/BridgeEventLoggerImplTest.java +++ b/rskj-core/src/test/java/co/rsk/peg/utils/BridgeEventLoggerImplTest.java @@ -19,13 +19,16 @@ import co.rsk.bitcoinj.core.*; import co.rsk.bitcoinj.script.ScriptBuilder; +import co.rsk.config.BridgeMainNetConstants; import co.rsk.config.BridgeRegTestConstants; import co.rsk.core.RskAddress; import co.rsk.crypto.Keccak256; import co.rsk.peg.*; +import co.rsk.peg.bitcoin.BitcoinTestUtils; import co.rsk.peg.pegin.RejectedPeginReason; import org.bouncycastle.util.encoders.Hex; import org.ethereum.config.blockchain.upgrades.ActivationConfig; +import org.ethereum.config.blockchain.upgrades.ActivationConfigsForTest; import org.ethereum.config.blockchain.upgrades.ConsensusRule; import org.ethereum.core.*; import org.ethereum.crypto.ECKey; @@ -36,6 +39,8 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import org.junit.jupiter.params.provider.ValueSource; import java.math.BigInteger; @@ -44,6 +49,7 @@ import java.util.LinkedList; import java.util.List; import java.util.stream.Collectors; +import java.util.stream.Stream; import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -174,21 +180,68 @@ void logUpdateCollections() { assertEvent(eventLogs, 0, BridgeEvents.UPDATE_COLLECTIONS.getEvent(), new Object[]{}, new Object[]{tx.getSender(signatureCache).toString()}); } - @Test - void logAddSignature() { - // Setup logAddSignature params - BtcECKey federatorPubKey = BtcECKey.fromPrivate(BigInteger.valueOf(2L)); + private static Stream logAddSignatureArgProvider() { + ActivationConfig.ForBlock fingerrootActivations = ActivationConfigsForTest.fingerroot500().forBlock(0); + ActivationConfig.ForBlock arrowHeadActivations = ActivationConfigsForTest.arrowhead600().forBlock(0); + + FederationMember multiKeyFedMember = new FederationMember( + BtcECKey.fromPrivate(BigInteger.valueOf(100)), + ECKey.fromPrivate(BigInteger.valueOf(200)), + ECKey.fromPrivate(BigInteger.valueOf(300)) + ); + ECKey rskKeyFromMultiFedMemberBtcKey = ECKey.fromPublicOnly(multiKeyFedMember.getBtcPublicKey().getPubKey()); + String wrongDerivedRskAddressFromMultiKeyFedMember = ByteUtil.toHexString(rskKeyFromMultiFedMemberBtcKey.getAddress()); + String derivedRskAddressFromMultiKeyFedMember = ByteUtil.toHexString(multiKeyFedMember.getRskPublicKey().getAddress()); + + Arguments before_arrowhead_should_log_wrong_rskAddress_for_multiKey_FedMember = Arguments.of(fingerrootActivations, multiKeyFedMember, wrongDerivedRskAddressFromMultiKeyFedMember); + Arguments after_arrowhead_should_log_correct_rskAddress_for_multiKey_FedMember = Arguments.of(arrowHeadActivations, multiKeyFedMember, derivedRskAddressFromMultiKeyFedMember); + + FederationMember singleKeyFedMember = new FederationMember( + BtcECKey.fromPrivate(BigInteger.valueOf(100)), + ECKey.fromPrivate(BigInteger.valueOf(100)), + ECKey.fromPrivate(BigInteger.valueOf(100)) + ); + ECKey rskKeyFromSingleFedMemberBtcKey = ECKey.fromPublicOnly(singleKeyFedMember.getBtcPublicKey().getPubKey()); + String derivedRskAddressFromSingleBtcKeyFedMember = ByteUtil.toHexString(rskKeyFromSingleFedMemberBtcKey.getAddress()); + String derivedRskAddressFromSingleRskKeyFedMember = ByteUtil.toHexString(singleKeyFedMember.getRskPublicKey().getAddress()); + + Arguments before_arrowhead_should_log_correct_rskAddress_for_single_FedMember = Arguments.of(fingerrootActivations, singleKeyFedMember, derivedRskAddressFromSingleBtcKeyFedMember); + Arguments after_arrowhead_should_log_correct_rskAddress_for_single_FedMember = Arguments.of(arrowHeadActivations, singleKeyFedMember, derivedRskAddressFromSingleRskKeyFedMember); + + return Stream.of( + before_arrowhead_should_log_wrong_rskAddress_for_multiKey_FedMember, + after_arrowhead_should_log_correct_rskAddress_for_multiKey_FedMember, + before_arrowhead_should_log_correct_rskAddress_for_single_FedMember, + after_arrowhead_should_log_correct_rskAddress_for_single_FedMember + ); + } + + @ParameterizedTest() + @MethodSource("logAddSignatureArgProvider") + void logAddSignature(ActivationConfig.ForBlock activations, FederationMember federationMember, RskAddress expectedRskAddress) { + // Arrange + BridgeMainNetConstants bridgeMainNetConstants = BridgeMainNetConstants.getInstance(); + BlockTxSignatureCache signatureCache = new BlockTxSignatureCache(new ReceivedTxSignatureCache()); + LinkedList eventLogs = new LinkedList<>(); + BridgeEventLogger eventLogger = new BridgeEventLoggerImpl(bridgeMainNetConstants, activations, eventLogs, signatureCache); + + + BtcECKey federatorBtcPubKey = federationMember.getBtcPublicKey(); + Keccak256 rskTxHash = PegTestUtils.createHash3(1); - when(btcTxMock.getHashAsString()).thenReturn("3e72fdbae7bbd103f08e876c765e3d5ba35db30ea46cb45ab52803f987ead9fb"); + + BtcTransaction btcTxMock = mock(BtcTransaction.class); + when(btcTxMock.getHash()).thenReturn(BitcoinTestUtils.createHash(1)); + when(btcTxMock.getHashAsString()).thenReturn(rskTxHash.toHexString()); // Act - eventLogger.logAddSignature(federatorPubKey, btcTxMock, rskTxHash.getBytes()); + eventLogger.logAddSignature(federationMember, btcTxMock, rskTxHash.getBytes()); + // Assert commonAssertLogs(eventLogs); assertTopics(3, eventLogs); - ECKey key = ECKey.fromPublicOnly(federatorPubKey.getPubKey()); - String federatorRskAddress = ByteUtil.toHexString(key.getAddress()); - assertEvent(eventLogs, 0, BridgeEvents.ADD_SIGNATURE.getEvent(), new Object[]{rskTxHash.getBytes(), federatorRskAddress}, new Object[]{federatorPubKey.getPubKey()}); + + assertEvent(eventLogs, 0, BridgeEvents.ADD_SIGNATURE.getEvent(), new Object[]{rskTxHash.getBytes(), ByteUtil.toHexString(expectedRskAddress.getBytes())}, new Object[]{federatorBtcPubKey.getPubKey()}); } @Test diff --git a/rskj-core/src/test/java/co/rsk/peg/utils/BridgeEventLoggerLegacyImplTest.java b/rskj-core/src/test/java/co/rsk/peg/utils/BridgeEventLoggerLegacyImplTest.java index dfc52e60c63..6094f732320 100644 --- a/rskj-core/src/test/java/co/rsk/peg/utils/BridgeEventLoggerLegacyImplTest.java +++ b/rskj-core/src/test/java/co/rsk/peg/utils/BridgeEventLoggerLegacyImplTest.java @@ -115,10 +115,11 @@ void testLogAddSignatureBeforeRskip146() { // Setup logAddSignature params BtcECKey federatorPubKey = BtcECKey.fromPrivate(BigInteger.valueOf(2L)); + FederationMember federationMember = FederationTestUtils.getFederationMemberWithKey(federatorPubKey); when(btcTxMock.getHashAsString()).thenReturn("3e72fdbae7bbd103f08e876c765e3d5ba35db30ea46cb45ab52803f987ead9fb"); // Act - eventLogger.logAddSignature(federatorPubKey, btcTxMock, rskTxHash.getBytes()); + eventLogger.logAddSignature(federationMember, btcTxMock, rskTxHash.getBytes()); // Assert log size Assertions.assertEquals(1, eventLogs.size()); @@ -147,8 +148,9 @@ void testLogAddSignatureBeforeRskip146() { void testLogAddSignatureAfterRskip146() { when(activations.isActive(ConsensusRule.RSKIP146)).thenReturn(true); BtcECKey federatorPublicKey = new BtcECKey(); + FederationMember federationMember = FederationTestUtils.getFederationMemberWithKey(federatorPublicKey); byte[] bytes = rskTxHash.getBytes(); - assertThrows(DeprecatedMethodCallException.class, () -> eventLogger.logAddSignature(federatorPublicKey, btcTxMock, bytes)); + assertThrows(DeprecatedMethodCallException.class, () -> eventLogger.logAddSignature(federationMember, btcTxMock, bytes)); } @Test diff --git a/rskj-core/src/test/java/org/ethereum/config/blockchain/upgrades/ActivationConfigsForTest.java b/rskj-core/src/test/java/org/ethereum/config/blockchain/upgrades/ActivationConfigsForTest.java index a80a3cdf0ca..f354cf6c380 100644 --- a/rskj-core/src/test/java/org/ethereum/config/blockchain/upgrades/ActivationConfigsForTest.java +++ b/rskj-core/src/test/java/org/ethereum/config/blockchain/upgrades/ActivationConfigsForTest.java @@ -182,7 +182,8 @@ private static List getArrowhead600Rskips() { ConsensusRule.RSKIP379, ConsensusRule.RSKIP398, ConsensusRule.RSKIP400, - ConsensusRule.RSKIP203 + ConsensusRule.RSKIP203, + ConsensusRule.RSKIP415 )); return rskips; From 7b407cddcdd6b749b6f0c1f0a210950c45e17e01 Mon Sep 17 00:00:00 2001 From: nathanieliov Date: Wed, 31 Jan 2024 01:01:35 -0400 Subject: [PATCH 07/18] Update getMemberByBtcPublicKey to compare public keys by byte array instead of using equal to the btcEcKey object itself --- rskj-core/src/main/java/co/rsk/peg/Federation.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/rskj-core/src/main/java/co/rsk/peg/Federation.java b/rskj-core/src/main/java/co/rsk/peg/Federation.java index f45579d18a8..38a82488b5a 100644 --- a/rskj-core/src/main/java/co/rsk/peg/Federation.java +++ b/rskj-core/src/main/java/co/rsk/peg/Federation.java @@ -65,7 +65,10 @@ public List getMembers() { } public Optional getMemberByBtcPublicKey(BtcECKey btcPublicKey) { - return members.stream().filter(federationMember -> federationMember.getBtcPublicKey().equals(btcPublicKey)).findAny(); + if (btcPublicKey == null){ + return Optional.empty(); + } + return members.stream().filter(federationMember -> Arrays.equals(federationMember.getBtcPublicKey().getPubKey(), btcPublicKey.getPubKey())).findAny(); } public List getBtcPublicKeys() { From 66763f05b8a1a21148fc53c7b092628db3f9a43d Mon Sep 17 00:00:00 2001 From: nathanieliov Date: Wed, 31 Jan 2024 09:31:57 -0400 Subject: [PATCH 08/18] Check if federation member was found before getting the value from the optional result when using getMemberByBtcPublicKey method --- rskj-core/src/main/java/co/rsk/peg/BridgeSupport.java | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/rskj-core/src/main/java/co/rsk/peg/BridgeSupport.java b/rskj-core/src/main/java/co/rsk/peg/BridgeSupport.java index 99b9367a330..91806937d83 100644 --- a/rskj-core/src/main/java/co/rsk/peg/BridgeSupport.java +++ b/rskj-core/src/main/java/co/rsk/peg/BridgeSupport.java @@ -1435,7 +1435,15 @@ public void addSignature(BtcECKey federatorPublicKey, List signatures, b } Federation federation = optionalFederation.get(); - FederationMember signingFederationMember = federation.getMemberByBtcPublicKey(federatorPublicKey).get(); + Optional federationMember = federation.getMemberByBtcPublicKey(federatorPublicKey); + if (!federationMember.isPresent()){ + logger.warn( + "Supplied federator public key {} does not belong to any of the federators.", + federatorPublicKey + ); + return; + } + FederationMember signingFederationMember = federationMember.get(); BtcTransaction btcTx = provider.getPegoutsWaitingForSignatures().get(new Keccak256(rskTxHash)); if (btcTx == null) { From 19b554f14d460972ede723c5c3205aefef365636 Mon Sep 17 00:00:00 2001 From: nathanieliov Date: Wed, 31 Jan 2024 22:28:54 -0400 Subject: [PATCH 09/18] - Reordered arrohead rskips. - Refactored logAddSignature method. - Refactored logAddSignature tests. - Renamed federatorPublicKey to federatorBtcPublicKey on processSigning method. - Updated getMemberByBtcPublicKey method to create a BtcEcKey with only the public key for then use it to find the federation member. --- .../main/java/co/rsk/peg/BridgeSupport.java | 14 ++-- .../src/main/java/co/rsk/peg/Federation.java | 3 +- .../rsk/peg/utils/BridgeEventLoggerImpl.java | 15 ++-- .../peg/utils/BridgeEventLoggerImplTest.java | 69 +++++++++++-------- .../upgrades/ActivationConfigsForTest.java | 2 +- 5 files changed, 60 insertions(+), 43 deletions(-) diff --git a/rskj-core/src/main/java/co/rsk/peg/BridgeSupport.java b/rskj-core/src/main/java/co/rsk/peg/BridgeSupport.java index 91806937d83..9afaa4a330c 100644 --- a/rskj-core/src/main/java/co/rsk/peg/BridgeSupport.java +++ b/rskj-core/src/main/java/co/rsk/peg/BridgeSupport.java @@ -1490,7 +1490,7 @@ private void processSigning( BtcTransaction btcTx, Federation federation) throws IOException { - BtcECKey federatorPublicKey = federatorMember.getBtcPublicKey(); + BtcECKey federatorBtcPublicKey = federatorMember.getBtcPublicKey(); // Build input hashes for signatures int numInputs = btcTx.getInputs().size(); @@ -1522,13 +1522,13 @@ private void processSigning( Sha256Hash sighash = sighashes.get(i); - if (!federatorPublicKey.verify(sighash, sig)) { + if (!federatorBtcPublicKey.verify(sighash, sig)) { logger.warn( "Signature {} {} is not valid for hash {} and public key {}", i, ByteUtil.toHexString(sig.encodeToDER()), sighash, - federatorPublicKey + federatorBtcPublicKey ); return; } @@ -1550,24 +1550,24 @@ private void processSigning( Script inputScript = input.getScriptSig(); boolean alreadySignedByThisFederator = BridgeUtils.isInputSignedByThisFederator( - federatorPublicKey, + federatorBtcPublicKey, sighash, input); // Sign the input if it wasn't already if (!alreadySignedByThisFederator) { try { - int sigIndex = inputScript.getSigInsertionIndex(sighash, federatorPublicKey); + int sigIndex = inputScript.getSigInsertionIndex(sighash, federatorBtcPublicKey); inputScript = ScriptBuilder.updateScriptWithSignature(inputScript, txSigs.get(i).encodeToBitcoin(), sigIndex, 1, 1); input.setScriptSig(inputScript); logger.debug("Tx input {} for tx {} signed.", i, new Keccak256(rskTxHash)); signed = true; } catch (IllegalStateException e) { Federation retiringFederation = getRetiringFederation(); - if (getActiveFederation().hasBtcPublicKey(federatorPublicKey)) { + if (getActiveFederation().hasBtcPublicKey(federatorBtcPublicKey)) { logger.debug("A member of the active federation is trying to sign a tx of the retiring one"); return; - } else if (retiringFederation != null && retiringFederation.hasBtcPublicKey(federatorPublicKey)) { + } else if (retiringFederation != null && retiringFederation.hasBtcPublicKey(federatorBtcPublicKey)) { logger.debug("A member of the retiring federation is trying to sign a tx of the active one"); return; } diff --git a/rskj-core/src/main/java/co/rsk/peg/Federation.java b/rskj-core/src/main/java/co/rsk/peg/Federation.java index 38a82488b5a..38cb23b8727 100644 --- a/rskj-core/src/main/java/co/rsk/peg/Federation.java +++ b/rskj-core/src/main/java/co/rsk/peg/Federation.java @@ -68,7 +68,8 @@ public Optional getMemberByBtcPublicKey(BtcECKey btcPublicKey) if (btcPublicKey == null){ return Optional.empty(); } - return members.stream().filter(federationMember -> Arrays.equals(federationMember.getBtcPublicKey().getPubKey(), btcPublicKey.getPubKey())).findAny(); + final BtcECKey btcPublicKeyOnly = BtcECKey.fromPublicOnly(btcPublicKey.getPubKey()); + return members.stream().filter(federationMember -> federationMember.getBtcPublicKey().equals(btcPublicKeyOnly)).findAny(); } public List getBtcPublicKeys() { diff --git a/rskj-core/src/main/java/co/rsk/peg/utils/BridgeEventLoggerImpl.java b/rskj-core/src/main/java/co/rsk/peg/utils/BridgeEventLoggerImpl.java index 3628b361b60..105c1431e1b 100644 --- a/rskj-core/src/main/java/co/rsk/peg/utils/BridgeEventLoggerImpl.java +++ b/rskj-core/src/main/java/co/rsk/peg/utils/BridgeEventLoggerImpl.java @@ -77,14 +77,17 @@ public void logUpdateCollections(Transaction rskTx) { @Override public void logAddSignature(FederationMember federationMember, BtcTransaction btcTx, byte[] rskTxHash) { + String federatorRskAddress = getFederatorRskAddress(federationMember); + logAddSignatureInSolidityFormat(rskTxHash, federatorRskAddress, federationMember.getBtcPublicKey()); + } + + private String getFederatorRskAddress(FederationMember federationMember){ if (shouldUseRskPublicKey()){ - String federatorRskAddress = ByteUtil.toHexString(federationMember.getRskPublicKey().getAddress()); - logAddSignatureInSolidityFormat(rskTxHash, federatorRskAddress, federationMember.getBtcPublicKey()); - } else { - ECKey ecKeyFromBtcPublicKey = ECKey.fromPublicOnly(federationMember.getBtcPublicKey().getPubKey()); - String federatorRskAddressFromBtcPublicKey = ByteUtil.toHexString(ecKeyFromBtcPublicKey.getAddress()); - logAddSignatureInSolidityFormat(rskTxHash, federatorRskAddressFromBtcPublicKey, federationMember.getBtcPublicKey()); + return ByteUtil.toHexString(federationMember.getRskPublicKey().getAddress()); } + + ECKey ecKeyFromBtcPublicKey = ECKey.fromPublicOnly(federationMember.getBtcPublicKey().getPubKey()); + return ByteUtil.toHexString(ecKeyFromBtcPublicKey.getAddress()); } private boolean shouldUseRskPublicKey() { diff --git a/rskj-core/src/test/java/co/rsk/peg/utils/BridgeEventLoggerImplTest.java b/rskj-core/src/test/java/co/rsk/peg/utils/BridgeEventLoggerImplTest.java index 863524a0e81..106da4e4a59 100644 --- a/rskj-core/src/test/java/co/rsk/peg/utils/BridgeEventLoggerImplTest.java +++ b/rskj-core/src/test/java/co/rsk/peg/utils/BridgeEventLoggerImplTest.java @@ -181,50 +181,62 @@ void logUpdateCollections() { } private static Stream logAddSignatureArgProvider() { - ActivationConfig.ForBlock fingerrootActivations = ActivationConfigsForTest.fingerroot500().forBlock(0); - ActivationConfig.ForBlock arrowHeadActivations = ActivationConfigsForTest.arrowhead600().forBlock(0); - - FederationMember multiKeyFedMember = new FederationMember( - BtcECKey.fromPrivate(BigInteger.valueOf(100)), - ECKey.fromPrivate(BigInteger.valueOf(200)), - ECKey.fromPrivate(BigInteger.valueOf(300)) - ); - ECKey rskKeyFromMultiFedMemberBtcKey = ECKey.fromPublicOnly(multiKeyFedMember.getBtcPublicKey().getPubKey()); - String wrongDerivedRskAddressFromMultiKeyFedMember = ByteUtil.toHexString(rskKeyFromMultiFedMemberBtcKey.getAddress()); - String derivedRskAddressFromMultiKeyFedMember = ByteUtil.toHexString(multiKeyFedMember.getRskPublicKey().getAddress()); - - Arguments before_arrowhead_should_log_wrong_rskAddress_for_multiKey_FedMember = Arguments.of(fingerrootActivations, multiKeyFedMember, wrongDerivedRskAddressFromMultiKeyFedMember); - Arguments after_arrowhead_should_log_correct_rskAddress_for_multiKey_FedMember = Arguments.of(arrowHeadActivations, multiKeyFedMember, derivedRskAddressFromMultiKeyFedMember); - FederationMember singleKeyFedMember = new FederationMember( BtcECKey.fromPrivate(BigInteger.valueOf(100)), ECKey.fromPrivate(BigInteger.valueOf(100)), ECKey.fromPrivate(BigInteger.valueOf(100)) ); - ECKey rskKeyFromSingleFedMemberBtcKey = ECKey.fromPublicOnly(singleKeyFedMember.getBtcPublicKey().getPubKey()); - String derivedRskAddressFromSingleBtcKeyFedMember = ByteUtil.toHexString(rskKeyFromSingleFedMemberBtcKey.getAddress()); - String derivedRskAddressFromSingleRskKeyFedMember = ByteUtil.toHexString(singleKeyFedMember.getRskPublicKey().getAddress()); - - Arguments before_arrowhead_should_log_correct_rskAddress_for_single_FedMember = Arguments.of(fingerrootActivations, singleKeyFedMember, derivedRskAddressFromSingleBtcKeyFedMember); - Arguments after_arrowhead_should_log_correct_rskAddress_for_single_FedMember = Arguments.of(arrowHeadActivations, singleKeyFedMember, derivedRskAddressFromSingleRskKeyFedMember); + FederationMember multiKeyFedMember = new FederationMember( + BtcECKey.fromPrivate(BigInteger.valueOf(100)), + ECKey.fromPrivate(BigInteger.valueOf(200)), + ECKey.fromPrivate(BigInteger.valueOf(300)) + ); return Stream.of( - before_arrowhead_should_log_wrong_rskAddress_for_multiKey_FedMember, - after_arrowhead_should_log_correct_rskAddress_for_multiKey_FedMember, - before_arrowhead_should_log_correct_rskAddress_for_single_FedMember, - after_arrowhead_should_log_correct_rskAddress_for_single_FedMember + Arguments.of(singleKeyFedMember, multiKeyFedMember) ); } @ParameterizedTest() @MethodSource("logAddSignatureArgProvider") - void logAddSignature(ActivationConfig.ForBlock activations, FederationMember federationMember, RskAddress expectedRskAddress) { + void logAddSignature_before_rskip415(FederationMember federationMember) { // Arrange + ActivationConfig.ForBlock fingerrootActivations = ActivationConfigsForTest.fingerroot500().forBlock(0); BridgeMainNetConstants bridgeMainNetConstants = BridgeMainNetConstants.getInstance(); BlockTxSignatureCache signatureCache = new BlockTxSignatureCache(new ReceivedTxSignatureCache()); LinkedList eventLogs = new LinkedList<>(); - BridgeEventLogger eventLogger = new BridgeEventLoggerImpl(bridgeMainNetConstants, activations, eventLogs, signatureCache); + BridgeEventLogger eventLogger = new BridgeEventLoggerImpl(bridgeMainNetConstants, fingerrootActivations, eventLogs, signatureCache); + + + BtcECKey federatorBtcPubKey = federationMember.getBtcPublicKey(); + + Keccak256 rskTxHash = PegTestUtils.createHash3(1); + + BtcTransaction btcTxMock = mock(BtcTransaction.class); + when(btcTxMock.getHash()).thenReturn(BitcoinTestUtils.createHash(1)); + when(btcTxMock.getHashAsString()).thenReturn(rskTxHash.toHexString()); + + // Act + eventLogger.logAddSignature(federationMember, btcTxMock, rskTxHash.getBytes()); + + // Assert + commonAssertLogs(eventLogs); + assertTopics(3, eventLogs); + ECKey federatorECKeyFromBtcPubKey = ECKey.fromPublicOnly(federationMember.getBtcPublicKey().getPubKey()); + String derivedRskAddress = ByteUtil.toHexString(federatorECKeyFromBtcPubKey.getAddress()); + assertEvent(eventLogs, 0, BridgeEvents.ADD_SIGNATURE.getEvent(), new Object[]{rskTxHash.getBytes(), derivedRskAddress}, new Object[]{federatorBtcPubKey.getPubKey()}); + } + + @ParameterizedTest() + @MethodSource("logAddSignatureArgProvider") + void logAddSignature_after_rskip415(FederationMember federationMember) { + // Arrange + ActivationConfig.ForBlock arrowheadActivations = ActivationConfigsForTest.arrowhead600().forBlock(0); + BridgeMainNetConstants bridgeMainNetConstants = BridgeMainNetConstants.getInstance(); + BlockTxSignatureCache signatureCache = new BlockTxSignatureCache(new ReceivedTxSignatureCache()); + LinkedList eventLogs = new LinkedList<>(); + BridgeEventLogger eventLogger = new BridgeEventLoggerImpl(bridgeMainNetConstants, arrowheadActivations, eventLogs, signatureCache); BtcECKey federatorBtcPubKey = federationMember.getBtcPublicKey(); @@ -241,7 +253,8 @@ void logAddSignature(ActivationConfig.ForBlock activations, FederationMember fed commonAssertLogs(eventLogs); assertTopics(3, eventLogs); - assertEvent(eventLogs, 0, BridgeEvents.ADD_SIGNATURE.getEvent(), new Object[]{rskTxHash.getBytes(), ByteUtil.toHexString(expectedRskAddress.getBytes())}, new Object[]{federatorBtcPubKey.getPubKey()}); + String derivedRskAddress = ByteUtil.toHexString(federationMember.getRskPublicKey().getAddress()); + assertEvent(eventLogs, 0, BridgeEvents.ADD_SIGNATURE.getEvent(), new Object[]{rskTxHash.getBytes(), derivedRskAddress}, new Object[]{federatorBtcPubKey.getPubKey()}); } @Test diff --git a/rskj-core/src/test/java/org/ethereum/config/blockchain/upgrades/ActivationConfigsForTest.java b/rskj-core/src/test/java/org/ethereum/config/blockchain/upgrades/ActivationConfigsForTest.java index f354cf6c380..14748d67147 100644 --- a/rskj-core/src/test/java/org/ethereum/config/blockchain/upgrades/ActivationConfigsForTest.java +++ b/rskj-core/src/test/java/org/ethereum/config/blockchain/upgrades/ActivationConfigsForTest.java @@ -178,11 +178,11 @@ private static List getFingerroot500Rskips() { private static List getArrowhead600Rskips() { List rskips = new ArrayList<>(); rskips.addAll(Arrays.asList( + ConsensusRule.RSKIP203, ConsensusRule.RSKIP376, ConsensusRule.RSKIP379, ConsensusRule.RSKIP398, ConsensusRule.RSKIP400, - ConsensusRule.RSKIP203, ConsensusRule.RSKIP415 )); From 542f4335d80c7b18a4feb11c446feb78f027b09e Mon Sep 17 00:00:00 2001 From: nathanieliov Date: Wed, 31 Jan 2024 23:51:03 -0400 Subject: [PATCH 10/18] Add tests for BridgeSupport.AddSignature method before and after RSKIP415 --- .../peg/BridgeSupportAddSignatureTest.java | 174 ++++++++++++++++++ 1 file changed, 174 insertions(+) diff --git a/rskj-core/src/test/java/co/rsk/peg/BridgeSupportAddSignatureTest.java b/rskj-core/src/test/java/co/rsk/peg/BridgeSupportAddSignatureTest.java index 941aed257b3..81a91db8311 100644 --- a/rskj-core/src/test/java/co/rsk/peg/BridgeSupportAddSignatureTest.java +++ b/rskj-core/src/test/java/co/rsk/peg/BridgeSupportAddSignatureTest.java @@ -5,9 +5,12 @@ import java.util.*; import co.rsk.config.BridgeConstants; +import co.rsk.config.BridgeMainNetConstants; 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.util.ByteUtil; import org.ethereum.util.RLP; import org.ethereum.util.RLPList; import org.ethereum.vm.LogInfo; @@ -543,6 +546,177 @@ void addSignatureMultipleInputsPartiallyValid() throws Exception { } } + @Test + void addSignature_beforeRSKIP415_should_derived_rskAddress_from_btcPublicKey() throws Exception { + // Arrange + BridgeMainNetConstants bridgeMainNetConstants = BridgeMainNetConstants.getInstance(); + NetworkParameters btcParams = bridgeMainNetConstants.getBtcParams(); + + ActivationConfig.ForBlock fingerrootActivations = ActivationConfigsForTest.fingerroot500().forBlock(0); + + List federationMembers = FederationTestUtils.getFederationMembersFromPks(150, 250, 350); + Federation federation = new StandardMultisigFederation( + federationMembers, + Instant.EPOCH, + 0, + btcParams + ); + + BridgeStorageProvider provider = mock(BridgeStorageProvider.class); + when(provider.getNewFederation()) + .thenReturn(federation); + LinkedList eventLogs = new LinkedList<>(); + BlockTxSignatureCache signatureCache = new BlockTxSignatureCache(new ReceivedTxSignatureCache()); + BridgeEventLogger eventLogger = new BridgeEventLoggerImpl(bridgeMainNetConstants, fingerrootActivations, eventLogs, signatureCache); + + // Build prev btc tx + BtcTransaction prevTx = new BtcTransaction(btcParams); + TransactionOutput prevOut = new TransactionOutput(btcParams, prevTx, Coin.FIFTY_COINS, federation.getAddress()); + prevTx.addOutput(prevOut); + + // Build btc tx to be signed + BtcTransaction btcTx = new BtcTransaction(btcParams); + btcTx.addInput(prevOut).setScriptSig(createBaseInputScriptThatSpendsFromTheFederation(federation)); + TransactionOutput output = new TransactionOutput(btcParams, btcTx, Coin.COIN, new BtcECKey().toAddress(btcParams)); + btcTx.addOutput(output); + + // Save btc tx to be signed + SortedMap pegoutWaitingForSignatures = new TreeMap<>(); + + final Keccak256 rskTxHash = createHash3(1); + pegoutWaitingForSignatures.put(rskTxHash, btcTx); + when(provider.getPegoutsWaitingForSignatures()) + .thenReturn(pegoutWaitingForSignatures); + + BridgeSupport bridgeSupport = bridgeSupportBuilder + .withBridgeConstants(bridgeMainNetConstants) + .withProvider(provider) + .withEventLogger(eventLogger) + .withBtcLockSenderProvider(new BtcLockSenderProvider()) + .withActivations(fingerrootActivations) + .build(); + + BtcECKey privateKeyToSignWith = BtcECKey.fromPrivate(BigInteger.valueOf(150)); + Optional federationMember = federation.getMemberByBtcPublicKey(privateKeyToSignWith); + assertTrue(federationMember.isPresent()); + FederationMember federationMemberToSignWith = federationMember.get(); + BtcECKey federatorBtcPubKey = federationMemberToSignWith.getBtcPublicKey(); + + Script inputScript = btcTx.getInputs().get(0).getScriptSig(); + List chunks = inputScript.getChunks(); + byte[] program = chunks.get(chunks.size() - 1).data; + Script redeemScript = new Script(program); + Sha256Hash sigHash = btcTx.hashForSignature(0, redeemScript, BtcTransaction.SigHash.ALL, false); + BtcECKey.ECDSASignature sig = privateKeyToSignWith.sign(sigHash); + List derEncodedSigs = Collections.singletonList(sig.encodeToDER()); + + // Act + bridgeSupport.addSignature(federationMemberToSignWith.getBtcPublicKey(), derEncodedSigs, rskTxHash.getBytes()); + + // Assert + commonAssertLogs(eventLogs); + assertTopics(3, eventLogs); + + ECKey federatorECKeyFromBtcPubKey = ECKey.fromPublicOnly(federationMemberToSignWith.getBtcPublicKey().getPubKey()); + String derivedRskAddress = ByteUtil.toHexString(federatorECKeyFromBtcPubKey.getAddress()); + + assertEvent(eventLogs, 0, BridgeEvents.ADD_SIGNATURE.getEvent(), new Object[]{rskTxHash.getBytes(), derivedRskAddress}, new Object[]{federatorBtcPubKey.getPubKey()}); + } + + @Test + void addSignature_afterRSKIP415_should_derived_rskAddress_from_rskPublicKey() throws Exception { + // Arrange + BridgeMainNetConstants bridgeMainNetConstants = BridgeMainNetConstants.getInstance(); + NetworkParameters btcParams = bridgeMainNetConstants.getBtcParams(); + + ActivationConfig.ForBlock arrowheadActivations = ActivationConfigsForTest.arrowhead600().forBlock(0); + + List federationMembers = FederationTestUtils.getFederationMembersFromPks(150, 250, 350); + Federation federation = new StandardMultisigFederation( + federationMembers, + Instant.EPOCH, + 0, + btcParams + ); + + BridgeStorageProvider provider = mock(BridgeStorageProvider.class); + when(provider.getNewFederation()) + .thenReturn(federation); + LinkedList eventLogs = new LinkedList<>(); + BlockTxSignatureCache signatureCache = new BlockTxSignatureCache(new ReceivedTxSignatureCache()); + BridgeEventLogger eventLogger = new BridgeEventLoggerImpl(bridgeMainNetConstants, arrowheadActivations, eventLogs, signatureCache); + + // Build prev btc tx + BtcTransaction prevTx = new BtcTransaction(btcParams); + TransactionOutput prevOut = new TransactionOutput(btcParams, prevTx, Coin.FIFTY_COINS, federation.getAddress()); + prevTx.addOutput(prevOut); + + // Build btc tx to be signed + BtcTransaction btcTx = new BtcTransaction(btcParams); + btcTx.addInput(prevOut).setScriptSig(createBaseInputScriptThatSpendsFromTheFederation(federation)); + TransactionOutput output = new TransactionOutput(btcParams, btcTx, Coin.COIN, new BtcECKey().toAddress(btcParams)); + btcTx.addOutput(output); + + // Save btc tx to be signed + SortedMap pegoutWaitingForSignatures = new TreeMap<>(); + + final Keccak256 rskTxHash = createHash3(1); + pegoutWaitingForSignatures.put(rskTxHash, btcTx); + when(provider.getPegoutsWaitingForSignatures()) + .thenReturn(pegoutWaitingForSignatures); + + BridgeSupport bridgeSupport = bridgeSupportBuilder + .withBridgeConstants(bridgeMainNetConstants) + .withProvider(provider) + .withEventLogger(eventLogger) + .withBtcLockSenderProvider(new BtcLockSenderProvider()) + .withActivations(arrowheadActivations) + .build(); + + BtcECKey privateKeyToSignWith = BtcECKey.fromPrivate(BigInteger.valueOf(150)); + Optional federationMember = federation.getMemberByBtcPublicKey(privateKeyToSignWith); + assertTrue(federationMember.isPresent()); + FederationMember federationMemberToSignWith = federationMember.get(); + BtcECKey federatorBtcPubKey = federationMemberToSignWith.getBtcPublicKey(); + + Script inputScript = btcTx.getInputs().get(0).getScriptSig(); + List chunks = inputScript.getChunks(); + byte[] program = chunks.get(chunks.size() - 1).data; + Script redeemScript = new Script(program); + Sha256Hash sigHash = btcTx.hashForSignature(0, redeemScript, BtcTransaction.SigHash.ALL, false); + BtcECKey.ECDSASignature sig = privateKeyToSignWith.sign(sigHash); + List derEncodedSigs = Collections.singletonList(sig.encodeToDER()); + + // Act + bridgeSupport.addSignature(federationMemberToSignWith.getBtcPublicKey(), derEncodedSigs, rskTxHash.getBytes()); + + // Assert + commonAssertLogs(eventLogs); + assertTopics(3, eventLogs); + + String derivedRskAddress = ByteUtil.toHexString(federationMemberToSignWith.getRskPublicKey().getAddress()); + assertEvent(eventLogs, 0, BridgeEvents.ADD_SIGNATURE.getEvent(), new Object[]{rskTxHash.getBytes(), derivedRskAddress}, new Object[]{federatorBtcPubKey.getPubKey()}); + } + + private static void assertEvent(List logs, int index, CallTransaction.Function event, Object[] topics, Object[] params) { + final LogInfo log = logs.get(index); + assertEquals(LogInfo.byteArrayToList(event.encodeEventTopics(topics)), log.getTopics()); + assertArrayEquals(event.encodeEventData(params), log.getData()); + } + + private void assertTopics(int topics, List logs) { + assertEquals(topics, logs.get(0).getTopics().size()); + } + + private void commonAssertLogs(List logs) { + assertEquals(1, logs.size()); + LogInfo entry = logs.get(0); + + // Assert address that made the log + assertEquals(PrecompiledContracts.BRIDGE_ADDR, new RskAddress(entry.getAddress())); + assertArrayEquals(PrecompiledContracts.BRIDGE_ADDR.getBytes(), entry.getAddress()); + } + /** * Helper method to test addSignature() with a valid federatorPublicKey parameter and both valid/invalid signatures * From f7b8869391b0c36716c8f9f19833424dd30a8030 Mon Sep 17 00:00:00 2001 From: nathanieliov Date: Thu, 1 Feb 2024 14:27:22 -0400 Subject: [PATCH 11/18] - Reorder logic for getFederatorPublicKey method, now default behaviour is the latest rskip. - Refactored logAddSignature and getFederatorPublicKey to simplify legacy and latest logic - Update addSignature log when none federation match supplied btc public key. --- .../main/java/co/rsk/peg/BridgeSupport.java | 6 +- .../rsk/peg/utils/BridgeEventLoggerImpl.java | 11 +- .../peg/BridgeSupportAddSignatureTest.java | 116 +++++------------- .../peg/utils/BridgeEventLoggerImplTest.java | 21 +++- 4 files changed, 58 insertions(+), 96 deletions(-) diff --git a/rskj-core/src/main/java/co/rsk/peg/BridgeSupport.java b/rskj-core/src/main/java/co/rsk/peg/BridgeSupport.java index 9afaa4a330c..fc9e8f0c332 100644 --- a/rskj-core/src/main/java/co/rsk/peg/BridgeSupport.java +++ b/rskj-core/src/main/java/co/rsk/peg/BridgeSupport.java @@ -1428,7 +1428,7 @@ public void addSignature(BtcECKey federatorPublicKey, List signatures, b Optional optionalFederation = getFederationFromPublicKey(federatorPublicKey); if (!optionalFederation.isPresent()) { logger.warn( - "Supplied federator public key {} does not belong to any of the federators.", + "[addSignature] Supplied federator btc public key {} does not belong to any of the federators.", federatorPublicKey ); return; @@ -1438,8 +1438,8 @@ public void addSignature(BtcECKey federatorPublicKey, List signatures, b Optional federationMember = federation.getMemberByBtcPublicKey(federatorPublicKey); if (!federationMember.isPresent()){ logger.warn( - "Supplied federator public key {} does not belong to any of the federators.", - federatorPublicKey + "[addSignature] Supplied federator btc public key {} doest not match any of the federator member btc public keys {}.", + federatorPublicKey, federation.getBtcPublicKeys() ); return; } diff --git a/rskj-core/src/main/java/co/rsk/peg/utils/BridgeEventLoggerImpl.java b/rskj-core/src/main/java/co/rsk/peg/utils/BridgeEventLoggerImpl.java index 105c1431e1b..70e1fbf0892 100644 --- a/rskj-core/src/main/java/co/rsk/peg/utils/BridgeEventLoggerImpl.java +++ b/rskj-core/src/main/java/co/rsk/peg/utils/BridgeEventLoggerImpl.java @@ -77,17 +77,16 @@ public void logUpdateCollections(Transaction rskTx) { @Override public void logAddSignature(FederationMember federationMember, BtcTransaction btcTx, byte[] rskTxHash) { - String federatorRskAddress = getFederatorRskAddress(federationMember); + String federatorRskAddress = ByteUtil.toHexString(getFederatorPublicKey(federationMember).getAddress());; logAddSignatureInSolidityFormat(rskTxHash, federatorRskAddress, federationMember.getBtcPublicKey()); } - private String getFederatorRskAddress(FederationMember federationMember){ - if (shouldUseRskPublicKey()){ - return ByteUtil.toHexString(federationMember.getRskPublicKey().getAddress()); + private ECKey getFederatorPublicKey(FederationMember federationMember) { + if (!shouldUseRskPublicKey()) { + return ECKey.fromPublicOnly(federationMember.getBtcPublicKey().getPubKey()); } - ECKey ecKeyFromBtcPublicKey = ECKey.fromPublicOnly(federationMember.getBtcPublicKey().getPubKey()); - return ByteUtil.toHexString(ecKeyFromBtcPublicKey.getAddress()); + return federationMember.getRskPublicKey(); } private boolean shouldUseRskPublicKey() { diff --git a/rskj-core/src/test/java/co/rsk/peg/BridgeSupportAddSignatureTest.java b/rskj-core/src/test/java/co/rsk/peg/BridgeSupportAddSignatureTest.java index 81a91db8311..cb29cd1a39a 100644 --- a/rskj-core/src/test/java/co/rsk/peg/BridgeSupportAddSignatureTest.java +++ b/rskj-core/src/test/java/co/rsk/peg/BridgeSupportAddSignatureTest.java @@ -3,6 +3,7 @@ import java.time.Instant; import java.math.BigInteger; import java.util.*; +import java.util.stream.Stream; import co.rsk.config.BridgeConstants; import co.rsk.config.BridgeMainNetConstants; @@ -41,6 +42,9 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import static co.rsk.peg.PegTestUtils.*; import static org.hamcrest.Matchers.hasItem; @@ -546,92 +550,44 @@ void addSignatureMultipleInputsPartiallyValid() throws Exception { } } - @Test - void addSignature_beforeRSKIP415_should_derived_rskAddress_from_btcPublicKey() throws Exception { - // Arrange - BridgeMainNetConstants bridgeMainNetConstants = BridgeMainNetConstants.getInstance(); - NetworkParameters btcParams = bridgeMainNetConstants.getBtcParams(); - + private static Stream addSignatureArgProvider() { ActivationConfig.ForBlock fingerrootActivations = ActivationConfigsForTest.fingerroot500().forBlock(0); + ActivationConfig.ForBlock arrowheadActivations = ActivationConfigsForTest.arrowhead600().forBlock(0); - List federationMembers = FederationTestUtils.getFederationMembersFromPks(150, 250, 350); - Federation federation = new StandardMultisigFederation( - federationMembers, - Instant.EPOCH, - 0, - btcParams - ); - - BridgeStorageProvider provider = mock(BridgeStorageProvider.class); - when(provider.getNewFederation()) - .thenReturn(federation); - LinkedList eventLogs = new LinkedList<>(); - BlockTxSignatureCache signatureCache = new BlockTxSignatureCache(new ReceivedTxSignatureCache()); - BridgeEventLogger eventLogger = new BridgeEventLoggerImpl(bridgeMainNetConstants, fingerrootActivations, eventLogs, signatureCache); - - // Build prev btc tx - BtcTransaction prevTx = new BtcTransaction(btcParams); - TransactionOutput prevOut = new TransactionOutput(btcParams, prevTx, Coin.FIFTY_COINS, federation.getAddress()); - prevTx.addOutput(prevOut); - - // Build btc tx to be signed - BtcTransaction btcTx = new BtcTransaction(btcParams); - btcTx.addInput(prevOut).setScriptSig(createBaseInputScriptThatSpendsFromTheFederation(federation)); - TransactionOutput output = new TransactionOutput(btcParams, btcTx, Coin.COIN, new BtcECKey().toAddress(btcParams)); - btcTx.addOutput(output); - - // Save btc tx to be signed - SortedMap pegoutWaitingForSignatures = new TreeMap<>(); - - final Keccak256 rskTxHash = createHash3(1); - pegoutWaitingForSignatures.put(rskTxHash, btcTx); - when(provider.getPegoutsWaitingForSignatures()) - .thenReturn(pegoutWaitingForSignatures); - - BridgeSupport bridgeSupport = bridgeSupportBuilder - .withBridgeConstants(bridgeMainNetConstants) - .withProvider(provider) - .withEventLogger(eventLogger) - .withBtcLockSenderProvider(new BtcLockSenderProvider()) - .withActivations(fingerrootActivations) - .build(); - - BtcECKey privateKeyToSignWith = BtcECKey.fromPrivate(BigInteger.valueOf(150)); - Optional federationMember = federation.getMemberByBtcPublicKey(privateKeyToSignWith); - assertTrue(federationMember.isPresent()); - FederationMember federationMemberToSignWith = federationMember.get(); - BtcECKey federatorBtcPubKey = federationMemberToSignWith.getBtcPublicKey(); - - Script inputScript = btcTx.getInputs().get(0).getScriptSig(); - List chunks = inputScript.getChunks(); - byte[] program = chunks.get(chunks.size() - 1).data; - Script redeemScript = new Script(program); - Sha256Hash sigHash = btcTx.hashForSignature(0, redeemScript, BtcTransaction.SigHash.ALL, false); - BtcECKey.ECDSASignature sig = privateKeyToSignWith.sign(sigHash); - List derEncodedSigs = Collections.singletonList(sig.encodeToDER()); - - // Act - bridgeSupport.addSignature(federationMemberToSignWith.getBtcPublicKey(), derEncodedSigs, rskTxHash.getBytes()); + BtcECKey btcKey = BtcECKey.fromPrivate(BigInteger.valueOf(350)); + ECKey rskKey = ECKey.fromPrivate(BigInteger.valueOf(350 + 1)); + ECKey mstKey = ECKey.fromPrivate(BigInteger.valueOf(350+2)); - // Assert - commonAssertLogs(eventLogs); - assertTopics(3, eventLogs); + FederationMember singleKeyFedMember = new FederationMember( + btcKey, + ECKey.fromPublicOnly(btcKey.getPubKey()), + ECKey.fromPublicOnly(btcKey.getPubKey()) + ); - ECKey federatorECKeyFromBtcPubKey = ECKey.fromPublicOnly(federationMemberToSignWith.getBtcPublicKey().getPubKey()); - String derivedRskAddress = ByteUtil.toHexString(federatorECKeyFromBtcPubKey.getAddress()); + FederationMember multiKeyFedMember = new FederationMember( + btcKey, + rskKey, + mstKey + ); - assertEvent(eventLogs, 0, BridgeEvents.ADD_SIGNATURE.getEvent(), new Object[]{rskTxHash.getBytes(), derivedRskAddress}, new Object[]{federatorBtcPubKey.getPubKey()}); + return Stream.of( + Arguments.of(fingerrootActivations, singleKeyFedMember, btcKey, ECKey.fromPublicOnly(singleKeyFedMember.getBtcPublicKey().getPubKey())), + Arguments.of(fingerrootActivations, multiKeyFedMember, btcKey, ECKey.fromPublicOnly(multiKeyFedMember.getBtcPublicKey().getPubKey())), + Arguments.of(arrowheadActivations, singleKeyFedMember, btcKey, singleKeyFedMember.getRskPublicKey()), + Arguments.of(arrowheadActivations, multiKeyFedMember, btcKey, multiKeyFedMember.getRskPublicKey()) + ); } - @Test - void addSignature_afterRSKIP415_should_derived_rskAddress_from_rskPublicKey() throws Exception { + @ParameterizedTest + @MethodSource("addSignatureArgProvider") + void addSignature(ActivationConfig.ForBlock activations, FederationMember federationMemberToSignWith, BtcECKey privateKeyToSignWith, ECKey expectedFederatorPublicKey) throws Exception { // Arrange BridgeMainNetConstants bridgeMainNetConstants = BridgeMainNetConstants.getInstance(); NetworkParameters btcParams = bridgeMainNetConstants.getBtcParams(); - ActivationConfig.ForBlock arrowheadActivations = ActivationConfigsForTest.arrowhead600().forBlock(0); + List federationMembers = FederationTestUtils.getFederationMembersFromPks(150, 250); + federationMembers.add(federationMemberToSignWith); - List federationMembers = FederationTestUtils.getFederationMembersFromPks(150, 250, 350); Federation federation = new StandardMultisigFederation( federationMembers, Instant.EPOCH, @@ -644,7 +600,7 @@ void addSignature_afterRSKIP415_should_derived_rskAddress_from_rskPublicKey() th .thenReturn(federation); LinkedList eventLogs = new LinkedList<>(); BlockTxSignatureCache signatureCache = new BlockTxSignatureCache(new ReceivedTxSignatureCache()); - BridgeEventLogger eventLogger = new BridgeEventLoggerImpl(bridgeMainNetConstants, arrowheadActivations, eventLogs, signatureCache); + BridgeEventLogger eventLogger = new BridgeEventLoggerImpl(bridgeMainNetConstants, activations, eventLogs, signatureCache); // Build prev btc tx BtcTransaction prevTx = new BtcTransaction(btcParams); @@ -670,13 +626,9 @@ void addSignature_afterRSKIP415_should_derived_rskAddress_from_rskPublicKey() th .withProvider(provider) .withEventLogger(eventLogger) .withBtcLockSenderProvider(new BtcLockSenderProvider()) - .withActivations(arrowheadActivations) + .withActivations(activations) .build(); - BtcECKey privateKeyToSignWith = BtcECKey.fromPrivate(BigInteger.valueOf(150)); - Optional federationMember = federation.getMemberByBtcPublicKey(privateKeyToSignWith); - assertTrue(federationMember.isPresent()); - FederationMember federationMemberToSignWith = federationMember.get(); BtcECKey federatorBtcPubKey = federationMemberToSignWith.getBtcPublicKey(); Script inputScript = btcTx.getInputs().get(0).getScriptSig(); @@ -694,8 +646,8 @@ void addSignature_afterRSKIP415_should_derived_rskAddress_from_rskPublicKey() th commonAssertLogs(eventLogs); assertTopics(3, eventLogs); - String derivedRskAddress = ByteUtil.toHexString(federationMemberToSignWith.getRskPublicKey().getAddress()); - assertEvent(eventLogs, 0, BridgeEvents.ADD_SIGNATURE.getEvent(), new Object[]{rskTxHash.getBytes(), derivedRskAddress}, new Object[]{federatorBtcPubKey.getPubKey()}); + String expectedDerivedRskAddress = ByteUtil.toHexString(expectedFederatorPublicKey.getAddress()); + assertEvent(eventLogs, 0, BridgeEvents.ADD_SIGNATURE.getEvent(), new Object[]{rskTxHash.getBytes(), expectedDerivedRskAddress}, new Object[]{federatorBtcPubKey.getPubKey()}); } private static void assertEvent(List logs, int index, CallTransaction.Function event, Object[] topics, Object[] params) { diff --git a/rskj-core/src/test/java/co/rsk/peg/utils/BridgeEventLoggerImplTest.java b/rskj-core/src/test/java/co/rsk/peg/utils/BridgeEventLoggerImplTest.java index 106da4e4a59..1c4e112236d 100644 --- a/rskj-core/src/test/java/co/rsk/peg/utils/BridgeEventLoggerImplTest.java +++ b/rskj-core/src/test/java/co/rsk/peg/utils/BridgeEventLoggerImplTest.java @@ -193,7 +193,8 @@ private static Stream logAddSignatureArgProvider() { ); return Stream.of( - Arguments.of(singleKeyFedMember, multiKeyFedMember) + Arguments.of(singleKeyFedMember), + Arguments.of(multiKeyFedMember) ); } @@ -224,8 +225,13 @@ void logAddSignature_before_rskip415(FederationMember federationMember) { assertTopics(3, eventLogs); ECKey federatorECKeyFromBtcPubKey = ECKey.fromPublicOnly(federationMember.getBtcPublicKey().getPubKey()); - String derivedRskAddress = ByteUtil.toHexString(federatorECKeyFromBtcPubKey.getAddress()); - assertEvent(eventLogs, 0, BridgeEvents.ADD_SIGNATURE.getEvent(), new Object[]{rskTxHash.getBytes(), derivedRskAddress}, new Object[]{federatorBtcPubKey.getPubKey()}); + String expectedRskAddress = ByteUtil.toHexString(federatorECKeyFromBtcPubKey.getAddress()); + + CallTransaction.Function bridgeEvent = BridgeEvents.ADD_SIGNATURE.getEvent(); + Object[] eventTopics = new Object[]{rskTxHash.getBytes(), expectedRskAddress}; + Object[] eventParams = new Object[]{federatorBtcPubKey.getPubKey()}; + + assertEvent(eventLogs, 0, bridgeEvent, eventTopics, eventParams); } @ParameterizedTest() @@ -253,8 +259,13 @@ void logAddSignature_after_rskip415(FederationMember federationMember) { commonAssertLogs(eventLogs); assertTopics(3, eventLogs); - String derivedRskAddress = ByteUtil.toHexString(federationMember.getRskPublicKey().getAddress()); - assertEvent(eventLogs, 0, BridgeEvents.ADD_SIGNATURE.getEvent(), new Object[]{rskTxHash.getBytes(), derivedRskAddress}, new Object[]{federatorBtcPubKey.getPubKey()}); + String expectedRskAddress = ByteUtil.toHexString(federationMember.getRskPublicKey().getAddress()); + + CallTransaction.Function bridgeEvent = BridgeEvents.ADD_SIGNATURE.getEvent(); + Object[] eventTopics = new Object[]{rskTxHash.getBytes(), expectedRskAddress}; + Object[] eventParams = new Object[]{federatorBtcPubKey.getPubKey()}; + + assertEvent(eventLogs, 0, bridgeEvent, eventTopics, eventParams); } @Test From e52364fd4bf941d25a694c94f0705c651276e93f Mon Sep 17 00:00:00 2001 From: nathanieliov Date: Fri, 2 Feb 2024 10:36:50 -0400 Subject: [PATCH 12/18] Put expected result hardcode for unit tests related to derived rsk address --- .../rsk/peg/utils/BridgeEventLoggerImpl.java | 2 +- .../peg/BridgeSupportAddSignatureTest.java | 23 +++--- .../peg/utils/BridgeEventLoggerImplTest.java | 73 ++++++------------- 3 files changed, 36 insertions(+), 62 deletions(-) diff --git a/rskj-core/src/main/java/co/rsk/peg/utils/BridgeEventLoggerImpl.java b/rskj-core/src/main/java/co/rsk/peg/utils/BridgeEventLoggerImpl.java index 70e1fbf0892..3472375b67e 100644 --- a/rskj-core/src/main/java/co/rsk/peg/utils/BridgeEventLoggerImpl.java +++ b/rskj-core/src/main/java/co/rsk/peg/utils/BridgeEventLoggerImpl.java @@ -77,7 +77,7 @@ public void logUpdateCollections(Transaction rskTx) { @Override public void logAddSignature(FederationMember federationMember, BtcTransaction btcTx, byte[] rskTxHash) { - String federatorRskAddress = ByteUtil.toHexString(getFederatorPublicKey(federationMember).getAddress());; + String federatorRskAddress = ByteUtil.toHexString(getFederatorPublicKey(federationMember).getAddress()); logAddSignatureInSolidityFormat(rskTxHash, federatorRskAddress, federationMember.getBtcPublicKey()); } diff --git a/rskj-core/src/test/java/co/rsk/peg/BridgeSupportAddSignatureTest.java b/rskj-core/src/test/java/co/rsk/peg/BridgeSupportAddSignatureTest.java index cb29cd1a39a..41ddd7aef87 100644 --- a/rskj-core/src/test/java/co/rsk/peg/BridgeSupportAddSignatureTest.java +++ b/rskj-core/src/test/java/co/rsk/peg/BridgeSupportAddSignatureTest.java @@ -11,7 +11,6 @@ import org.ethereum.config.blockchain.upgrades.ConsensusRule; import org.ethereum.core.*; import org.ethereum.crypto.ECKey; -import org.ethereum.util.ByteUtil; import org.ethereum.util.RLP; import org.ethereum.util.RLPList; import org.ethereum.vm.LogInfo; @@ -554,9 +553,12 @@ private static Stream addSignatureArgProvider() { ActivationConfig.ForBlock fingerrootActivations = ActivationConfigsForTest.fingerroot500().forBlock(0); ActivationConfig.ForBlock arrowheadActivations = ActivationConfigsForTest.arrowhead600().forBlock(0); - BtcECKey btcKey = BtcECKey.fromPrivate(BigInteger.valueOf(350)); - ECKey rskKey = ECKey.fromPrivate(BigInteger.valueOf(350 + 1)); - ECKey mstKey = ECKey.fromPrivate(BigInteger.valueOf(350+2)); + BtcECKey btcKey = BtcECKey.fromPrivate(Hex.decode("000000000000000000000000000000000000000000000000000000000000015e")); // 350 + ECKey rskKey = ECKey.fromPrivate(Hex.decode("000000000000000000000000000000000000000000000000000000000000015f")); // 351 + ECKey mstKey = ECKey.fromPrivate(Hex.decode("0000000000000000000000000000000000000000000000000000000000000160")); // 352 + + String addressDerivedFromBtcKey = "dbc29273d4de3d5645e308c7e629d28d4499b3d3"; + String addressDerivedFromRskKey = "74891a05ad4d7ec87c1cffe9bd00bb4e1382b586"; FederationMember singleKeyFedMember = new FederationMember( btcKey, @@ -571,16 +573,16 @@ private static Stream addSignatureArgProvider() { ); return Stream.of( - Arguments.of(fingerrootActivations, singleKeyFedMember, btcKey, ECKey.fromPublicOnly(singleKeyFedMember.getBtcPublicKey().getPubKey())), - Arguments.of(fingerrootActivations, multiKeyFedMember, btcKey, ECKey.fromPublicOnly(multiKeyFedMember.getBtcPublicKey().getPubKey())), - Arguments.of(arrowheadActivations, singleKeyFedMember, btcKey, singleKeyFedMember.getRskPublicKey()), - Arguments.of(arrowheadActivations, multiKeyFedMember, btcKey, multiKeyFedMember.getRskPublicKey()) + Arguments.of(fingerrootActivations, singleKeyFedMember, btcKey, addressDerivedFromBtcKey), + Arguments.of(fingerrootActivations, multiKeyFedMember, btcKey, addressDerivedFromBtcKey), + Arguments.of(arrowheadActivations, singleKeyFedMember, btcKey, addressDerivedFromBtcKey), // Given this is a single key fed member, the rsk address is equal to the one obtained from btc key + Arguments.of(arrowheadActivations, multiKeyFedMember, btcKey, addressDerivedFromRskKey) ); } @ParameterizedTest @MethodSource("addSignatureArgProvider") - void addSignature(ActivationConfig.ForBlock activations, FederationMember federationMemberToSignWith, BtcECKey privateKeyToSignWith, ECKey expectedFederatorPublicKey) throws Exception { + void addSignature(ActivationConfig.ForBlock activations, FederationMember federationMemberToSignWith, BtcECKey privateKeyToSignWith, String expectedRskAddress) throws Exception { // Arrange BridgeMainNetConstants bridgeMainNetConstants = BridgeMainNetConstants.getInstance(); NetworkParameters btcParams = bridgeMainNetConstants.getBtcParams(); @@ -646,8 +648,7 @@ void addSignature(ActivationConfig.ForBlock activations, FederationMember federa commonAssertLogs(eventLogs); assertTopics(3, eventLogs); - String expectedDerivedRskAddress = ByteUtil.toHexString(expectedFederatorPublicKey.getAddress()); - assertEvent(eventLogs, 0, BridgeEvents.ADD_SIGNATURE.getEvent(), new Object[]{rskTxHash.getBytes(), expectedDerivedRskAddress}, new Object[]{federatorBtcPubKey.getPubKey()}); + assertEvent(eventLogs, 0, BridgeEvents.ADD_SIGNATURE.getEvent(), new Object[]{rskTxHash.getBytes(), expectedRskAddress}, new Object[]{federatorBtcPubKey.getPubKey()}); } private static void assertEvent(List logs, int index, CallTransaction.Function event, Object[] topics, Object[] params) { diff --git a/rskj-core/src/test/java/co/rsk/peg/utils/BridgeEventLoggerImplTest.java b/rskj-core/src/test/java/co/rsk/peg/utils/BridgeEventLoggerImplTest.java index 1c4e112236d..9aa61dc3e1d 100644 --- a/rskj-core/src/test/java/co/rsk/peg/utils/BridgeEventLoggerImplTest.java +++ b/rskj-core/src/test/java/co/rsk/peg/utils/BridgeEventLoggerImplTest.java @@ -32,7 +32,6 @@ import org.ethereum.config.blockchain.upgrades.ConsensusRule; import org.ethereum.core.*; import org.ethereum.crypto.ECKey; -import org.ethereum.util.ByteUtil; import org.ethereum.vm.LogInfo; import org.ethereum.vm.PrecompiledContracts; import org.junit.jupiter.api.Assertions; @@ -181,68 +180,44 @@ void logUpdateCollections() { } private static Stream logAddSignatureArgProvider() { + ActivationConfig.ForBlock fingerrootActivations = ActivationConfigsForTest.fingerroot500().forBlock(0); + ActivationConfig.ForBlock arrowheadActivations = ActivationConfigsForTest.arrowhead600().forBlock(0); + + BtcECKey btcKey = BtcECKey.fromPrivate(Hex.decode("000000000000000000000000000000000000000000000000000000000000015e")); // 350 + ECKey rskKey = ECKey.fromPrivate(Hex.decode("000000000000000000000000000000000000000000000000000000000000015f")); // 351 + ECKey mstKey = ECKey.fromPrivate(Hex.decode("0000000000000000000000000000000000000000000000000000000000000160")); // 352 + + String addressDerivedFromBtcKey = "dbc29273d4de3d5645e308c7e629d28d4499b3d3"; + String addressDerivedFromRskKey = "74891a05ad4d7ec87c1cffe9bd00bb4e1382b586"; + FederationMember singleKeyFedMember = new FederationMember( - BtcECKey.fromPrivate(BigInteger.valueOf(100)), - ECKey.fromPrivate(BigInteger.valueOf(100)), - ECKey.fromPrivate(BigInteger.valueOf(100)) + btcKey, + ECKey.fromPublicOnly(btcKey.getPubKey()), + ECKey.fromPublicOnly(btcKey.getPubKey()) ); + FederationMember multiKeyFedMember = new FederationMember( - BtcECKey.fromPrivate(BigInteger.valueOf(100)), - ECKey.fromPrivate(BigInteger.valueOf(200)), - ECKey.fromPrivate(BigInteger.valueOf(300)) + btcKey, + rskKey, + mstKey ); return Stream.of( - Arguments.of(singleKeyFedMember), - Arguments.of(multiKeyFedMember) + Arguments.of(fingerrootActivations, singleKeyFedMember, addressDerivedFromBtcKey), + Arguments.of(fingerrootActivations, multiKeyFedMember, addressDerivedFromBtcKey), + Arguments.of(arrowheadActivations, singleKeyFedMember, addressDerivedFromBtcKey), // Given this is a single key fed member, the rsk address is equal to the one obtained from btc key + Arguments.of(arrowheadActivations, multiKeyFedMember, addressDerivedFromRskKey) ); } @ParameterizedTest() @MethodSource("logAddSignatureArgProvider") - void logAddSignature_before_rskip415(FederationMember federationMember) { - // Arrange - ActivationConfig.ForBlock fingerrootActivations = ActivationConfigsForTest.fingerroot500().forBlock(0); - BridgeMainNetConstants bridgeMainNetConstants = BridgeMainNetConstants.getInstance(); - BlockTxSignatureCache signatureCache = new BlockTxSignatureCache(new ReceivedTxSignatureCache()); - LinkedList eventLogs = new LinkedList<>(); - BridgeEventLogger eventLogger = new BridgeEventLoggerImpl(bridgeMainNetConstants, fingerrootActivations, eventLogs, signatureCache); - - - BtcECKey federatorBtcPubKey = federationMember.getBtcPublicKey(); - - Keccak256 rskTxHash = PegTestUtils.createHash3(1); - - BtcTransaction btcTxMock = mock(BtcTransaction.class); - when(btcTxMock.getHash()).thenReturn(BitcoinTestUtils.createHash(1)); - when(btcTxMock.getHashAsString()).thenReturn(rskTxHash.toHexString()); - - // Act - eventLogger.logAddSignature(federationMember, btcTxMock, rskTxHash.getBytes()); - - // Assert - commonAssertLogs(eventLogs); - assertTopics(3, eventLogs); - - ECKey federatorECKeyFromBtcPubKey = ECKey.fromPublicOnly(federationMember.getBtcPublicKey().getPubKey()); - String expectedRskAddress = ByteUtil.toHexString(federatorECKeyFromBtcPubKey.getAddress()); - - CallTransaction.Function bridgeEvent = BridgeEvents.ADD_SIGNATURE.getEvent(); - Object[] eventTopics = new Object[]{rskTxHash.getBytes(), expectedRskAddress}; - Object[] eventParams = new Object[]{federatorBtcPubKey.getPubKey()}; - - assertEvent(eventLogs, 0, bridgeEvent, eventTopics, eventParams); - } - - @ParameterizedTest() - @MethodSource("logAddSignatureArgProvider") - void logAddSignature_after_rskip415(FederationMember federationMember) { + void logAddSignature(ActivationConfig.ForBlock activations, FederationMember federationMember, String expectedRskAddress) { // Arrange - ActivationConfig.ForBlock arrowheadActivations = ActivationConfigsForTest.arrowhead600().forBlock(0); BridgeMainNetConstants bridgeMainNetConstants = BridgeMainNetConstants.getInstance(); BlockTxSignatureCache signatureCache = new BlockTxSignatureCache(new ReceivedTxSignatureCache()); LinkedList eventLogs = new LinkedList<>(); - BridgeEventLogger eventLogger = new BridgeEventLoggerImpl(bridgeMainNetConstants, arrowheadActivations, eventLogs, signatureCache); + BridgeEventLogger eventLogger = new BridgeEventLoggerImpl(bridgeMainNetConstants, activations, eventLogs, signatureCache); BtcECKey federatorBtcPubKey = federationMember.getBtcPublicKey(); @@ -259,8 +234,6 @@ void logAddSignature_after_rskip415(FederationMember federationMember) { commonAssertLogs(eventLogs); assertTopics(3, eventLogs); - String expectedRskAddress = ByteUtil.toHexString(federationMember.getRskPublicKey().getAddress()); - CallTransaction.Function bridgeEvent = BridgeEvents.ADD_SIGNATURE.getEvent(); Object[] eventTopics = new Object[]{rskTxHash.getBytes(), expectedRskAddress}; Object[] eventParams = new Object[]{federatorBtcPubKey.getPubKey()}; From 21683e6de788bc136b708ceb715dfb908aee0c0e Mon Sep 17 00:00:00 2001 From: Marcos Irisarri <53787863+marcos-iov@users.noreply.github.com> Date: Fri, 2 Feb 2024 13:41:04 -0300 Subject: [PATCH 13/18] Update BridgeEventLoggerImpl.java --- .../src/main/java/co/rsk/peg/utils/BridgeEventLoggerImpl.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/rskj-core/src/main/java/co/rsk/peg/utils/BridgeEventLoggerImpl.java b/rskj-core/src/main/java/co/rsk/peg/utils/BridgeEventLoggerImpl.java index 3472375b67e..cfddb9a2008 100644 --- a/rskj-core/src/main/java/co/rsk/peg/utils/BridgeEventLoggerImpl.java +++ b/rskj-core/src/main/java/co/rsk/peg/utils/BridgeEventLoggerImpl.java @@ -77,7 +77,8 @@ public void logUpdateCollections(Transaction rskTx) { @Override public void logAddSignature(FederationMember federationMember, BtcTransaction btcTx, byte[] rskTxHash) { - String federatorRskAddress = ByteUtil.toHexString(getFederatorPublicKey(federationMember).getAddress()); + ECKey federatorPublicKey = getFederatorPublicKey(federationMember); + String federatorRskAddress = ByteUtil.toHexString(federatorPublicKey.getAddress()); logAddSignatureInSolidityFormat(rskTxHash, federatorRskAddress, federationMember.getBtcPublicKey()); } From b834f08ecfaaa19cc72e32501a3d499744936d58 Mon Sep 17 00:00:00 2001 From: nathanieliov Date: Tue, 6 Feb 2024 23:57:56 -0400 Subject: [PATCH 14/18] Fixed misused of activation config passing wrong execution block number. --- .../src/main/java/co/rsk/remasc/Remasc.java | 33 ++++++++++--------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/rskj-core/src/main/java/co/rsk/remasc/Remasc.java b/rskj-core/src/main/java/co/rsk/remasc/Remasc.java index 4ea53fbb837..28b215b2c82 100644 --- a/rskj-core/src/main/java/co/rsk/remasc/Remasc.java +++ b/rskj-core/src/main/java/co/rsk/remasc/Remasc.java @@ -51,7 +51,7 @@ public class Remasc { private static final Logger logger = LoggerFactory.getLogger(Remasc.class); private final Constants constants; - private final ActivationConfig activationConfig; + private final ActivationConfig.ForBlock activations; private final Repository repository; private final BlockStore blockStore; private final RemascConfig remascConstants; @@ -82,7 +82,7 @@ public Remasc( this.provider = new RemascStorageProvider(repository, contractAddress); this.feesPayer = new RemascFeesPayer(repository, contractAddress); this.constants = constants; - this.activationConfig = activationConfig; + this.activations = activationConfig.forBlock(executionBlock.getNumber()); } public void save() { @@ -113,11 +113,11 @@ void processMinersFees() { throw new RemascInvalidInvocationException("Invoked Remasc outside last tx of the block"); } - long blockNbr = executionBlock.getNumber(); + long executionBlockNumber = executionBlock.getNumber(); - long processingBlockNumber = blockNbr - remascConstants.getMaturity(); - if (processingBlockNumber < 1 ) { - logger.debug("First block has not reached maturity yet, current block is {}", blockNbr); + long candidateBlockNumberToReward = executionBlockNumber - remascConstants.getMaturity(); + if (candidateBlockNumberToReward < 1 ) { + logger.debug("First block has not reached maturity yet, current block is {}", executionBlockNumber); return; } @@ -149,20 +149,20 @@ void processMinersFees() { rewardBalance = rewardBalance.add(processingBlockReward); provider.setRewardBalance(rewardBalance); - if (processingBlockNumber - remascConstants.getSyntheticSpan() < 0 ) { + if (candidateBlockNumberToReward - remascConstants.getSyntheticSpan() < 0 ) { logger.debug("First block has not reached maturity+syntheticSpan yet, current block is {}", executionBlock.getNumber()); return; } - List siblings = getSiblingsToReward(descendantsBlocks, processingBlockNumber); + List siblings = getSiblingsToReward(descendantsBlocks, candidateBlockNumberToReward); boolean previousBrokenSelectionRule = provider.getBrokenSelectionRule(); boolean brokenSelectionRule = SelectionRule.isBrokenSelectionRule(processingBlockHeader, siblings); provider.setBrokenSelectionRule(!siblings.isEmpty() && brokenSelectionRule); // Takes from rewardBalance this block's height reward. Coin syntheticReward = rewardBalance.divide(BigInteger.valueOf(remascConstants.getSyntheticSpan())); - boolean isRskip85Enabled = activationConfig.isActive(ConsensusRule.RSKIP85, blockNbr); - if (isRskip85Enabled) { + + if (shouldRewardBeAboveMinimumPayableGas()) { BigInteger minimumPayableGas = constants.getMinimumPayableGas(); Coin minPayableFees = executionBlock.getMinimumGasPrice().multiply(minimumPayableGas); if (syntheticReward.compareTo(minPayableFees) < 0) { @@ -197,22 +197,23 @@ void processMinersFees() { } } + private boolean shouldRewardBeAboveMinimumPayableGas() { + return activations.isActive(ConsensusRule.RSKIP85); + } + RskAddress getRskLabsAddress() { - boolean isRskip218Enabled = activationConfig.isActive(ConsensusRule.RSKIP218, executionBlock.getNumber()); + boolean isRskip218Enabled = activations.isActive(ConsensusRule.RSKIP218); return isRskip218Enabled ? remascConstants.getRskLabsAddressRskip218() : remascConstants.getRskLabsAddress(); } private Coin payToFederation(Constants constants, Block processingBlock, BlockHeader processingBlockHeader, Coin syntheticReward) { - - ActivationConfig.ForBlock activations = activationConfig.forBlock(processingBlock.getNumber()); - BridgeConstants bridgeConstants = constants.getBridgeConstants(); BridgeStorageProvider bridgeStorageProvider = new BridgeStorageProvider( repository, PrecompiledContracts.BRIDGE_ADDR, bridgeConstants, - activations + activations ); FederationSupport federationSupport = new FederationSupport(bridgeConstants, bridgeStorageProvider, processingBlock, activations); @@ -227,7 +228,7 @@ private Coin payToFederation(Constants constants, Block processingBlock, BlockHe Coin payToFederator = payAndRemainderToFederator[0]; Coin restToLastFederator = payAndRemainderToFederator[1]; - if (activations.isActive(ConsensusRule.RSKIP85)) { + if (shouldRewardBeAboveMinimumPayableGas()) { BigInteger minimumFederatorPayableGas = constants.getFederatorMinimumPayableGas(); Coin minPayableFederatorFees = executionBlock.getMinimumGasPrice().multiply(minimumFederatorPayableGas); if (payToFederator.compareTo(minPayableFederatorFees) < 0) { From 76e7a5f39fcdb85f02618bc0210b6d0006a6b9b3 Mon Sep 17 00:00:00 2001 From: nathanieliov Date: Wed, 7 Feb 2024 08:38:36 -0400 Subject: [PATCH 15/18] Refactored RemascRskAddressActivationTest --- .../RemascRskAddressActivationTest.java | 23 +++++++++++++------ 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/rskj-core/src/test/java/co/rsk/remasc/RemascRskAddressActivationTest.java b/rskj-core/src/test/java/co/rsk/remasc/RemascRskAddressActivationTest.java index ffbce207cdb..60dde908511 100644 --- a/rskj-core/src/test/java/co/rsk/remasc/RemascRskAddressActivationTest.java +++ b/rskj-core/src/test/java/co/rsk/remasc/RemascRskAddressActivationTest.java @@ -53,31 +53,40 @@ void testActivation() { final RemascConfig remascConfig = spy(new RemascConfigFactory(RemascContract.REMASC_CONFIG) .createRemascConfig("regtest")); - final Remasc remasc = new Remasc(Constants.regtest(), activationConfig, repositoryMock, - blockStoreMock, remascConfig, txMock, PrecompiledContracts.REMASC_ADDR, blockMock, logs); - when(remascConfig.getRskLabsAddress()).thenReturn(rskLabsAddress); when(remascConfig.getRskLabsAddressRskip218()).thenReturn(rskLabsAddressRskip218); - when(activationConfig.isActive(ConsensusRule.RSKIP218, 1)).thenReturn(false); - when(activationConfig.isActive(ConsensusRule.RSKIP218, 2)).thenReturn(true); + ActivationConfig.ForBlock activationsBeforeRSKIP218 = mock(ActivationConfig.ForBlock.class); + when(activationsBeforeRSKIP218.isActive(ConsensusRule.RSKIP218)).thenReturn(false); + + ActivationConfig.ForBlock activationsAfterRSKIP218 = mock(ActivationConfig.ForBlock.class); + when(activationsAfterRSKIP218.isActive(ConsensusRule.RSKIP218)).thenReturn(true); + + when(activationConfig.forBlock(1)).thenReturn(activationsBeforeRSKIP218); + when(activationConfig.forBlock(2)).thenReturn(activationsAfterRSKIP218); when(blockMock.getNumber()).thenReturn(1L); + Remasc remasc = new Remasc(Constants.regtest(), activationConfig, repositoryMock, + blockStoreMock, remascConfig, txMock, PrecompiledContracts.REMASC_ADDR, blockMock, logs); + RskAddress actualAddress = remasc.getRskLabsAddress(); - Assertions.assertEquals(rskLabsAddress, actualAddress); Assertions.assertEquals(1L, blockMock.getNumber()); + Assertions.assertEquals(rskLabsAddress, actualAddress); + Assertions.assertFalse(activationConfig.isActive(ConsensusRule.RSKIP218, blockMock.getNumber())); verify(remascConfig).getRskLabsAddress(); when(blockMock.getNumber()).thenReturn(2L); + remasc = new Remasc(Constants.regtest(), activationConfig, repositoryMock, + blockStoreMock, remascConfig, txMock, PrecompiledContracts.REMASC_ADDR, blockMock, logs); + actualAddress = remasc.getRskLabsAddress(); Assertions.assertEquals(rskLabsAddressRskip218, actualAddress); Assertions.assertEquals(2L, blockMock.getNumber()); - Assertions.assertTrue(activationConfig.isActive(ConsensusRule.RSKIP218, blockMock.getNumber())); verify(remascConfig).getRskLabsAddressRskip218(); } } From f4547e6100bc931150f89702d0f6cf821da65bbc Mon Sep 17 00:00:00 2001 From: nathanieliov Date: Wed, 7 Feb 2024 16:09:20 -0400 Subject: [PATCH 16/18] Added unit tests for processMinersFees method, testing before and after rskip85 --- .../src/main/java/co/rsk/remasc/Remasc.java | 6 +- .../test/java/co/rsk/remasc/RemascTest.java | 164 ++++++++++++++++++ 2 files changed, 167 insertions(+), 3 deletions(-) create mode 100644 rskj-core/src/test/java/co/rsk/remasc/RemascTest.java diff --git a/rskj-core/src/main/java/co/rsk/remasc/Remasc.java b/rskj-core/src/main/java/co/rsk/remasc/Remasc.java index 28b215b2c82..e59291f21d3 100644 --- a/rskj-core/src/main/java/co/rsk/remasc/Remasc.java +++ b/rskj-core/src/main/java/co/rsk/remasc/Remasc.java @@ -180,7 +180,7 @@ void processMinersFees() { Coin payToRskLabs = syntheticReward.divide(BigInteger.valueOf(remascConstants.getRskLabsDivisor())); feesPayer.payMiningFees(processingBlockHeader.getHash().getBytes(), payToRskLabs, rskLabsAddress, logs); syntheticReward = syntheticReward.subtract(payToRskLabs); - Coin payToFederation = payToFederation(constants, processingBlock, processingBlockHeader, syntheticReward); + Coin payToFederation = payToFederation(constants, processingBlock, syntheticReward); syntheticReward = syntheticReward.subtract(payToFederation); if (!siblings.isEmpty()) { @@ -206,7 +206,7 @@ RskAddress getRskLabsAddress() { return isRskip218Enabled ? remascConstants.getRskLabsAddressRskip218() : remascConstants.getRskLabsAddress(); } - private Coin payToFederation(Constants constants, Block processingBlock, BlockHeader processingBlockHeader, Coin syntheticReward) { + private Coin payToFederation(Constants constants, Block processingBlock, Coin syntheticReward) { BridgeConstants bridgeConstants = constants.getBridgeConstants(); BridgeStorageProvider bridgeStorageProvider = new BridgeStorageProvider( @@ -222,7 +222,7 @@ private Coin payToFederation(Constants constants, Block processingBlock, BlockHe Coin federationReward = syntheticReward.divide(BigInteger.valueOf(remascConstants.getFederationDivisor())); Coin payToFederation = provider.getFederationBalance().add(federationReward); - byte[] processingBlockHash = processingBlockHeader.getHash().getBytes(); + byte[] processingBlockHash = processingBlock.getHeader().getHash().getBytes(); int nfederators = federationProvider.getFederationSize(); Coin[] payAndRemainderToFederator = payToFederation.divideAndRemainder(BigInteger.valueOf(nfederators)); Coin payToFederator = payAndRemainderToFederator[0]; diff --git a/rskj-core/src/test/java/co/rsk/remasc/RemascTest.java b/rskj-core/src/test/java/co/rsk/remasc/RemascTest.java new file mode 100644 index 00000000000..60184870ff6 --- /dev/null +++ b/rskj-core/src/test/java/co/rsk/remasc/RemascTest.java @@ -0,0 +1,164 @@ +package co.rsk.remasc; + +import co.rsk.config.RemascConfig; +import co.rsk.config.RemascConfigFactory; +import co.rsk.core.Coin; +import co.rsk.core.RskAddress; +import co.rsk.peg.PegTestUtils; +import org.ethereum.config.Constants; +import org.ethereum.config.blockchain.upgrades.ActivationConfig; +import org.ethereum.config.blockchain.upgrades.ActivationConfigsForTest; +import org.ethereum.core.Block; +import org.ethereum.core.BlockHeader; +import org.ethereum.core.Repository; +import org.ethereum.db.BlockStore; +import org.ethereum.vm.DataWord; +import org.ethereum.vm.LogInfo; +import org.ethereum.vm.PrecompiledContracts; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +import java.math.BigInteger; +import java.util.ArrayList; +import java.util.List; +import java.util.stream.Stream; + +import static org.mockito.Mockito.*; +import static org.mockito.Mockito.when; + +class RemascTest { + + private final static ActivationConfig genesisActivations = ActivationConfigsForTest.genesis(); + private final static ActivationConfig orchidActivations = ActivationConfigsForTest.orchid(); + private final static Constants mainnet = Constants.mainnet(); + private final static Coin minimumGasPrice = Coin.valueOf(100L); + + Repository repository; + BlockStore blockStore; + Block nextBlockToReward; + + private final static RemascConfig remascConfig = new RemascConfigFactory(RemascContract.REMASC_CONFIG).createRemascConfig("main"); + + RemascTransaction executionTx; + + RskAddress remascAddr; + + Block executionBlock; + + BlockHeader blockHeader; + + List logs; + + List blocks; + + DataWord rewardBalanceKey = DataWord.fromString("rewardBalance"); + + @BeforeEach + void setUp() { + repository = mock(Repository.class); + blockStore = mock(BlockStore.class); + nextBlockToReward = mock(Block.class); + long blockNumber = 3992L; + when(nextBlockToReward.getParentHash()).thenReturn(PegTestUtils.createHash3((int) (blockNumber-1))); + when(nextBlockToReward.getHash()).thenReturn(PegTestUtils.createHash3((int) blockNumber)); + when(nextBlockToReward.getNumber()).thenReturn(blockNumber); + when(blockStore.getBlockByHash(nextBlockToReward.getParentHash().getBytes())).thenReturn(nextBlockToReward); + + when(blockStore.getBlockAtDepthStartingAt(anyLong(), any(byte[].class))).thenReturn(nextBlockToReward); + + executionTx = mock(RemascTransaction.class); + + remascAddr = PrecompiledContracts.REMASC_ADDR; + + executionBlock = mock(Block.class); + when(executionBlock.getMinimumGasPrice()).thenReturn(minimumGasPrice); + when(executionBlock.getNumber()).thenReturn(4100L); + when(executionBlock.getHash()).thenReturn(PegTestUtils.createHash3(4100)); + + blockHeader = mock(BlockHeader.class); + when(executionBlock.getHeader()).thenReturn(blockHeader); + when(executionBlock.getParentHash()).thenReturn(PegTestUtils.createHash3(1)); + + logs = new ArrayList<>(); + } + + private void mockBlockStore(Coin feesPerBlock) { + blocks = new ArrayList<>(); + int uncleGenerationLimit = mainnet.getUncleGenerationLimit(); + + for (int i = 0; i < uncleGenerationLimit + 1; i++) { + Block currentBlock = mock(Block.class); + int currentBlockNumber = (int) (nextBlockToReward.getNumber() - i); + when(currentBlock.getParentHash()).thenReturn(PegTestUtils.createHash3(currentBlockNumber - 1)); + when(currentBlock.getHash()).thenReturn(PegTestUtils.createHash3(currentBlockNumber)); + when(currentBlock.getNumber()).thenReturn((long) currentBlockNumber); + + BlockHeader blockHeader = mock(BlockHeader.class); + + when(blockHeader.getPaidFees()).thenReturn(feesPerBlock); + when(blockHeader.getHash()).thenReturn(PegTestUtils.createHash3(currentBlockNumber)); + when(blockHeader.getCoinbase()).thenReturn(PegTestUtils.createRandomRskAddress()); + + when(currentBlock.getHeader()).thenReturn(blockHeader); + + when(blockStore.getBlockByHash(currentBlock.getHash().getBytes())).thenReturn(currentBlock); + blocks.add(currentBlock); + } + } + + private static Stream processMinersFeesArgProvider() { + Coin minimumPayableGas = Coin.valueOf(mainnet.getMinimumPayableGas().longValue()); + Coin minPayableFees = minimumGasPrice.multiply(minimumPayableGas.asBigInteger()); + + Coin equalToMinPayableFees = minPayableFees.multiply(BigInteger.valueOf(remascConfig.getSyntheticSpan())); + Coin belowMinPayableFees = equalToMinPayableFees.subtract(Coin.valueOf(1L)); + + return Stream.of( + Arguments.of(genesisActivations, belowMinPayableFees, true), + Arguments.of(orchidActivations, belowMinPayableFees, false), + Arguments.of(orchidActivations, equalToMinPayableFees, true) + ); + } + + @ParameterizedTest + @MethodSource("processMinersFeesArgProvider") + void test_processMinersFees(ActivationConfig activationConfig, Coin feesPerBlock, boolean success) { + mockBlockStore(feesPerBlock); + + // Arrange + Remasc remasc = new Remasc( + mainnet, + activationConfig, + repository, + blockStore, + remascConfig, + executionTx, + remascAddr, + executionBlock, + logs + ); + + // Act + remasc.processMinersFees(); + remasc.save(); + + // Assert + Coin syntheticReward = feesPerBlock.divide(BigInteger.valueOf(remascConfig.getSyntheticSpan())); + Coin expectedRemascPayment = feesPerBlock.subtract(syntheticReward); + Coin expectedRskLabPayment = syntheticReward.divide(BigInteger.valueOf(remascConfig.getRskLabsDivisor())); + RskAddress iovLabsAddress = remasc.getRskLabsAddress(); + + if (success) { + verify(this.repository, times(1)).addStorageRow(remascAddr, rewardBalanceKey, DataWord.valueOf(expectedRemascPayment.getBytes())); + verify(this.repository, times(1)).addBalance(remascAddr, expectedRskLabPayment.negate()); + verify(this.repository, times(1)).addBalance(iovLabsAddress, expectedRskLabPayment); + } else { + verify(this.repository, times(1)).addStorageRow(remascAddr, rewardBalanceKey, DataWord.valueOf(feesPerBlock.getBytes())); + verify(this.repository, never()).addBalance(any(), any()); + verify(this.repository, never()).addBalance(any(),any()); + } + } +} From 171193ba5dc3dd4041eaf7c9d4e6226a2646577b Mon Sep 17 00:00:00 2001 From: nathanieliov Date: Thu, 8 Feb 2024 23:09:19 -0400 Subject: [PATCH 17/18] - Added RSKIP134 as part of papyrus200 rskips in ActivationConfigsForTest class - Updated tests to consider locking cap RSKIP134 - Removed unused imports. --- ...ridgeStorageProviderPegoutTxIndexTests.java | 18 ++++++++++++++---- ...ridgeSupportRegisterBtcTransactionTest.java | 8 ++++++-- .../java/co/rsk/peg/PowpegMigrationTest.java | 3 +++ .../test/java/co/rsk/remasc/RemascTest.java | 1 - .../upgrades/ActivationConfigsForTest.java | 1 + 5 files changed, 24 insertions(+), 7 deletions(-) diff --git a/rskj-core/src/test/java/co/rsk/peg/BridgeStorageProviderPegoutTxIndexTests.java b/rskj-core/src/test/java/co/rsk/peg/BridgeStorageProviderPegoutTxIndexTests.java index 2f5b725423b..35ae28447b7 100644 --- a/rskj-core/src/test/java/co/rsk/peg/BridgeStorageProviderPegoutTxIndexTests.java +++ b/rskj-core/src/test/java/co/rsk/peg/BridgeStorageProviderPegoutTxIndexTests.java @@ -18,9 +18,11 @@ import org.mockito.Mockito; import java.io.IOException; +import java.util.Collections; import java.util.stream.Stream; import static co.rsk.peg.BridgeStorageIndexKey.PEGOUT_TX_SIG_HASH; +import static org.ethereum.config.blockchain.upgrades.ConsensusRule.RSKIP134; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; @@ -285,8 +287,8 @@ void hasPegoutTxSigHash_passing_existing_sigHash() throws IOException { void setPegoutTxSigHash_null(boolean isRskip379HardForkActive) throws IOException { // Arrange ActivationConfig.ForBlock activations = isRskip379HardForkActive ? - ActivationConfigsForTest.arrowhead600().forBlock(0) : - ActivationConfigsForTest.fingerroot500().forBlock(0); + getArrowHeadActivationExceptLockingCap() : + getFingerrootActivationsExceptLockingCap(); Repository repository = mock(Repository.class); @@ -319,8 +321,8 @@ void setPegoutTxSigHash_null(boolean isRskip379HardForkActive) throws IOExceptio void setPegoutTxSigHash_non_null(boolean isRskip379HardForkActive, Sha256Hash sigHash) throws IOException { // Arrange ActivationConfig.ForBlock activations = isRskip379HardForkActive ? - ActivationConfigsForTest.arrowhead600().forBlock(0) : - ActivationConfigsForTest.fingerroot500().forBlock(0); + getArrowHeadActivationExceptLockingCap() : + getFingerrootActivationsExceptLockingCap(); Repository repository = mock(Repository.class); BridgeStorageProvider provider = new BridgeStorageProvider( @@ -360,6 +362,14 @@ void setPegoutTxSigHash_non_null(boolean isRskip379HardForkActive, Sha256Hash si } } + private ActivationConfig.ForBlock getFingerrootActivationsExceptLockingCap() { + return ActivationConfigsForTest.fingerroot500(Collections.singletonList(RSKIP134)).forBlock(0); + } + + private ActivationConfig.ForBlock getArrowHeadActivationExceptLockingCap() { + return ActivationConfigsForTest.arrowhead600(Collections.singletonList(RSKIP134)).forBlock(0); + } + @Test void setPegoutTxSigHash_passing_existing() throws IOException { // Arrange diff --git a/rskj-core/src/test/java/co/rsk/peg/BridgeSupportRegisterBtcTransactionTest.java b/rskj-core/src/test/java/co/rsk/peg/BridgeSupportRegisterBtcTransactionTest.java index 143ba162990..a200fe18340 100644 --- a/rskj-core/src/test/java/co/rsk/peg/BridgeSupportRegisterBtcTransactionTest.java +++ b/rskj-core/src/test/java/co/rsk/peg/BridgeSupportRegisterBtcTransactionTest.java @@ -35,6 +35,7 @@ import org.ethereum.core.Block; import org.ethereum.core.BlockTxSignatureCache; import org.ethereum.core.ReceivedTxSignatureCache; +import org.ethereum.core.Repository; import org.ethereum.core.SignatureCache; import org.ethereum.core.Transaction; import org.ethereum.crypto.ECKey; @@ -503,9 +504,14 @@ private PartialMerkleTree createPmtAndMockBlockStore(BtcTransaction btcTransacti } private BridgeSupport buildBridgeSupport(ActivationConfig.ForBlock activations) { + Repository repository = mock(Repository.class); + when(repository.getBalance(PrecompiledContracts.BRIDGE_ADDR)).thenReturn(co.rsk.core.Coin.fromBitcoin(bridgeMainnetConstants.getMaxRbtc())); + when(provider.getLockingCap()).thenReturn(bridgeMainnetConstants.getMaxRbtc()); + return new BridgeSupportBuilder() .withBtcBlockStoreFactory(mockFactory) .withBridgeConstants(bridgeMainnetConstants) + .withRepository(repository) .withProvider(provider) .withActivations(activations) .withSignatureCache(signatureCache) @@ -535,8 +541,6 @@ void registering_btc_transaction_sending_funds_to_unknown_address( btcTransaction.addInput(BitcoinTestUtils.createHash(1), FIRST_OUTPUT_INDEX, ScriptBuilder.createInputScript(null, senderBtcKey)); - - Coin amountToSend = shouldSendAmountBelowMinimum ? belowMinimumPeginTxValue : minimumPeginTxValue; btcTransaction.addOutput(amountToSend, userAddress); diff --git a/rskj-core/src/test/java/co/rsk/peg/PowpegMigrationTest.java b/rskj-core/src/test/java/co/rsk/peg/PowpegMigrationTest.java index c682f43c700..972fdb68309 100644 --- a/rskj-core/src/test/java/co/rsk/peg/PowpegMigrationTest.java +++ b/rskj-core/src/test/java/co/rsk/peg/PowpegMigrationTest.java @@ -88,6 +88,9 @@ private void testChangePowpeg( activations ); + repository.addBalance(PrecompiledContracts.BRIDGE_ADDR, co.rsk.core.Coin.fromBitcoin(bridgeConstants.getMaxRbtc())); + bridgeStorageProvider.setLockingCap(bridgeConstants.getMaxRbtc()); + BtcBlockStoreWithCache.Factory btcBlockStoreFactory = new RepositoryBtcBlockStoreWithCache.Factory( bridgeConstants.getBtcParams(), 100, diff --git a/rskj-core/src/test/java/co/rsk/remasc/RemascTest.java b/rskj-core/src/test/java/co/rsk/remasc/RemascTest.java index 60184870ff6..896d2744285 100644 --- a/rskj-core/src/test/java/co/rsk/remasc/RemascTest.java +++ b/rskj-core/src/test/java/co/rsk/remasc/RemascTest.java @@ -16,7 +16,6 @@ import org.ethereum.vm.LogInfo; import org.ethereum.vm.PrecompiledContracts; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; diff --git a/rskj-core/src/test/java/org/ethereum/config/blockchain/upgrades/ActivationConfigsForTest.java b/rskj-core/src/test/java/org/ethereum/config/blockchain/upgrades/ActivationConfigsForTest.java index 14748d67147..dfaaeeb5889 100644 --- a/rskj-core/src/test/java/org/ethereum/config/blockchain/upgrades/ActivationConfigsForTest.java +++ b/rskj-core/src/test/java/org/ethereum/config/blockchain/upgrades/ActivationConfigsForTest.java @@ -95,6 +95,7 @@ private static List getTwoToThreeRskips() { private static List getPapyrus200Rskips() { List rskips = new ArrayList<>(); rskips.addAll(Arrays.asList( + ConsensusRule.RSKIP134, ConsensusRule.RSKIP137, ConsensusRule.RSKIP140, ConsensusRule.RSKIP143, From c8d41fe50a4a12c344f48bc5e8f8ccd260ff379b Mon Sep 17 00:00:00 2001 From: nathanieliov Date: Fri, 9 Feb 2024 10:43:14 -0400 Subject: [PATCH 18/18] Refactored tests to remove logic that do not include locking cap rskip in certain tests --- .../co/rsk/peg/BridgeStorageProvider.java | 2 +- ...idgeStorageProviderPegoutTxIndexTests.java | 20 ++++++------------- 2 files changed, 7 insertions(+), 15 deletions(-) diff --git a/rskj-core/src/main/java/co/rsk/peg/BridgeStorageProvider.java b/rskj-core/src/main/java/co/rsk/peg/BridgeStorageProvider.java index 256059905fa..cf60e67005c 100644 --- a/rskj-core/src/main/java/co/rsk/peg/BridgeStorageProvider.java +++ b/rskj-core/src/main/java/co/rsk/peg/BridgeStorageProvider.java @@ -956,7 +956,7 @@ public void setPegoutTxSigHash(Sha256Hash sigHash) { pegoutTxSigHashes.add(sigHash); } - private void savePegoutTxSigHashes() { + protected void savePegoutTxSigHashes() { if (!activations.isActive(RSKIP379) || pegoutTxSigHashes == null) { return; } diff --git a/rskj-core/src/test/java/co/rsk/peg/BridgeStorageProviderPegoutTxIndexTests.java b/rskj-core/src/test/java/co/rsk/peg/BridgeStorageProviderPegoutTxIndexTests.java index 35ae28447b7..668bc9427d5 100644 --- a/rskj-core/src/test/java/co/rsk/peg/BridgeStorageProviderPegoutTxIndexTests.java +++ b/rskj-core/src/test/java/co/rsk/peg/BridgeStorageProviderPegoutTxIndexTests.java @@ -287,8 +287,8 @@ void hasPegoutTxSigHash_passing_existing_sigHash() throws IOException { void setPegoutTxSigHash_null(boolean isRskip379HardForkActive) throws IOException { // Arrange ActivationConfig.ForBlock activations = isRskip379HardForkActive ? - getArrowHeadActivationExceptLockingCap() : - getFingerrootActivationsExceptLockingCap(); + ActivationConfigsForTest.arrowhead600().forBlock(0) : + ActivationConfigsForTest.fingerroot500().forBlock(0); Repository repository = mock(Repository.class); @@ -301,7 +301,7 @@ void setPegoutTxSigHash_null(boolean isRskip379HardForkActive) throws IOExceptio // Act provider.setPegoutTxSigHash(null); - provider.save(); + provider.savePegoutTxSigHashes(); // Assert verify(repository, never()).getStorageBytes( @@ -321,8 +321,8 @@ void setPegoutTxSigHash_null(boolean isRskip379HardForkActive) throws IOExceptio void setPegoutTxSigHash_non_null(boolean isRskip379HardForkActive, Sha256Hash sigHash) throws IOException { // Arrange ActivationConfig.ForBlock activations = isRskip379HardForkActive ? - getArrowHeadActivationExceptLockingCap() : - getFingerrootActivationsExceptLockingCap(); + ActivationConfigsForTest.arrowhead600().forBlock(0) : + ActivationConfigsForTest.fingerroot500().forBlock(0); Repository repository = mock(Repository.class); BridgeStorageProvider provider = new BridgeStorageProvider( @@ -334,7 +334,7 @@ void setPegoutTxSigHash_non_null(boolean isRskip379HardForkActive, Sha256Hash si // Act provider.setPegoutTxSigHash(sigHash); - provider.save(); + provider.savePegoutTxSigHashes(); // Assert if (isRskip379HardForkActive) { @@ -362,14 +362,6 @@ void setPegoutTxSigHash_non_null(boolean isRskip379HardForkActive, Sha256Hash si } } - private ActivationConfig.ForBlock getFingerrootActivationsExceptLockingCap() { - return ActivationConfigsForTest.fingerroot500(Collections.singletonList(RSKIP134)).forBlock(0); - } - - private ActivationConfig.ForBlock getArrowHeadActivationExceptLockingCap() { - return ActivationConfigsForTest.arrowhead600(Collections.singletonList(RSKIP134)).forBlock(0); - } - @Test void setPegoutTxSigHash_passing_existing() throws IOException { // Arrange