From 2d7bd49a1c39914686f0bebabd177c11cc89eebc Mon Sep 17 00:00:00 2001 From: Marcos Date: Fri, 16 Aug 2024 11:45:13 -0300 Subject: [PATCH 1/7] Config fix wip --- .../java/co/rsk/federate/FedNodeRunner.java | 40 ++++++++++--------- 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/src/main/java/co/rsk/federate/FedNodeRunner.java b/src/main/java/co/rsk/federate/FedNodeRunner.java index f6924aaa4..3e1534234 100644 --- a/src/main/java/co/rsk/federate/FedNodeRunner.java +++ b/src/main/java/co/rsk/federate/FedNodeRunner.java @@ -128,24 +128,10 @@ public void run() throws Exception { LOGGER.debug("[run] Starting RSK"); signer = buildSigner(); - try { - PowHSMConfig powHsmConfig = new PowHSMConfig( - config.signerConfig(BTC.getId())); - - HSMClientProtocol protocol = - hsmClientProtocolFactory.buildHSMClientProtocolFromConfig(powHsmConfig); - - int hsmVersion = protocol.getVersion(); - LOGGER.debug("[run] Using HSM version {}", hsmVersion); - - if (HSMVersion.isPowHSM(hsmVersion)) { - hsmBookkeepingClient = buildBookKeepingClient( - protocol, powHsmConfig); - hsmBookkeepingService = buildBookKeepingService( - hsmBookkeepingClient, powHsmConfig); - } - } catch (SignerException e) { - LOGGER.info("[run] PowHSM config was not found", e); + SignerConfig btcSignerConfig = config.signerConfig(BTC.getId()); + if (btcSignerConfig != null && btcSignerConfig.getType().equals("hsm")) { + LOGGER.info("[run] BTC signer is an HSM"); + startBookkeepingServices(); } if (!this.checkFederateRequirements()) { @@ -167,6 +153,24 @@ public void run() throws Exception { LOGGER.info("[run] RSK address: {}", Hex.toHexString(this.member.getRskPublicKey().getAddress())); } + private void startBookkeepingServices() throws SignerException, HSMClientException { + PowHSMConfig powHsmConfig = new PowHSMConfig( + config.signerConfig(BTC.getId())); + + HSMClientProtocol protocol = + hsmClientProtocolFactory.buildHSMClientProtocolFromConfig(powHsmConfig); + + int hsmVersion = protocol.getVersion(); + LOGGER.debug("[run] Using HSM version {}", hsmVersion); + + if (HSMVersion.isPowHSM(hsmVersion)) { + hsmBookkeepingClient = buildBookKeepingClient( + protocol, powHsmConfig); + hsmBookkeepingService = buildBookKeepingService( + hsmBookkeepingClient, powHsmConfig); + } + } + private void configureFederatorSupport() throws SignerException { BtcECKey btcPublicKey = signer.getPublicKey(BTC.getKeyId()).toBtcKey(); ECKey rskPublicKey = signer.getPublicKey(RSK.getKeyId()).toEthKey(); From b9dec38942075ff7c8c1e12b30ddb93f9a6f8d8c Mon Sep 17 00:00:00 2001 From: Antonio Pancorbo <48168255+apancorb@users.noreply.github.com> Date: Thu, 22 Aug 2024 14:43:37 +0000 Subject: [PATCH 2/7] feat(config): create signer type enum --- .../java/co/rsk/federate/FedNodeRunner.java | 4 +-- .../federate/signing/ECDSASignerFactory.java | 7 +++--- .../federate/signing/config/SignerConfig.java | 6 ++--- .../federate/signing/config/SignerType.java | 25 +++++++++++++++++++ .../signing/hsm/config/PowHSMConfig.java | 4 +-- .../PowpegNodeSystemPropertiesTest.java | 1 + .../signing/ECDSASignerFactoryTest.java | 2 +- .../signing/hsm/config/PowHSMConfigTest.java | 3 ++- 8 files changed, 40 insertions(+), 12 deletions(-) create mode 100644 src/main/java/co/rsk/federate/signing/config/SignerType.java diff --git a/src/main/java/co/rsk/federate/FedNodeRunner.java b/src/main/java/co/rsk/federate/FedNodeRunner.java index 3e1534234..233bc4ed5 100644 --- a/src/main/java/co/rsk/federate/FedNodeRunner.java +++ b/src/main/java/co/rsk/federate/FedNodeRunner.java @@ -32,6 +32,7 @@ import co.rsk.federate.btcreleaseclient.BtcReleaseClientStorageSynchronizer; import co.rsk.federate.config.PowpegNodeSystemProperties; import co.rsk.federate.signing.config.SignerConfig; +import co.rsk.federate.signing.config.SignerType; import co.rsk.federate.signing.hsm.config.PowHSMConfig; import co.rsk.federate.io.*; import co.rsk.federate.log.BtcLogMonitor; @@ -129,8 +130,7 @@ public void run() throws Exception { signer = buildSigner(); SignerConfig btcSignerConfig = config.signerConfig(BTC.getId()); - if (btcSignerConfig != null && btcSignerConfig.getType().equals("hsm")) { - LOGGER.info("[run] BTC signer is an HSM"); + if (btcSignerConfig != null && btcSignerConfig.getSignerType() == SignerType.HSM) { startBookkeepingServices(); } diff --git a/src/main/java/co/rsk/federate/signing/ECDSASignerFactory.java b/src/main/java/co/rsk/federate/signing/ECDSASignerFactory.java index 1a406bc22..2a4503b06 100644 --- a/src/main/java/co/rsk/federate/signing/ECDSASignerFactory.java +++ b/src/main/java/co/rsk/federate/signing/ECDSASignerFactory.java @@ -19,6 +19,7 @@ package co.rsk.federate.signing; import co.rsk.federate.signing.config.SignerConfig; +import co.rsk.federate.signing.config.SignerType; import co.rsk.federate.signing.hsm.config.PowHSMConfig; import co.rsk.federate.signing.hsm.SignerException; import co.rsk.federate.signing.hsm.client.HSMClientProtocol; @@ -34,15 +35,15 @@ public ECDSASigner buildFromConfig(SignerConfig config) throws SignerException { if (config == null) { throw new SignerException("'signers' entry not found in config file."); } - String type = config.getType(); + SignerType type = config.getSignerType(); logger.debug("[buildFromConfig] SignerConfig type {}", type); switch (type) { - case "keyFile": + case KEYFILE: return new ECDSASignerFromFileKey( new KeyId(config.getId()), config.getConfig().getString("path") ); - case "hsm": + case HSM: try { PowHSMConfig powHSMConfig = new PowHSMConfig(config); HSMClientProtocol hsmClientProtocol = diff --git a/src/main/java/co/rsk/federate/signing/config/SignerConfig.java b/src/main/java/co/rsk/federate/signing/config/SignerConfig.java index dd48ca4d8..39bd33ebc 100644 --- a/src/main/java/co/rsk/federate/signing/config/SignerConfig.java +++ b/src/main/java/co/rsk/federate/signing/config/SignerConfig.java @@ -7,12 +7,12 @@ public class SignerConfig { private static final String SIGNER_TYPE_PATH = "type"; private final String id; - private final String type; + private final SignerType type; private final Config config; public SignerConfig(String keyId, Config config) { this.id = keyId; - this.type = config.getString(SIGNER_TYPE_PATH); + this.type = SignerType.fromConfigValue(config.getString(SIGNER_TYPE_PATH)); this.config = config.withoutPath(SIGNER_TYPE_PATH); } @@ -20,7 +20,7 @@ public String getId() { return id; } - public String getType() { + public SignerType getSignerType() { return type; } diff --git a/src/main/java/co/rsk/federate/signing/config/SignerType.java b/src/main/java/co/rsk/federate/signing/config/SignerType.java new file mode 100644 index 000000000..ec8b5cf8a --- /dev/null +++ b/src/main/java/co/rsk/federate/signing/config/SignerType.java @@ -0,0 +1,25 @@ +package co.rsk.federate.signing.config; + +public enum SignerType { + KEYFILE("keyFile"), + HSM("hsm"); + + private final String type; + + SignerType(String signerType) { + this.type = signerType; + } + + public static SignerType fromConfigValue(String configValue) { + for (SignerType signerType : values()) { + if (signerType.getType().equals(configValue)) { + return signerType; + } + } + throw new RuntimeException(String.format("Unsupported signer type: %s", configValue)); + } + + public String getType() { + return type; + } +} diff --git a/src/main/java/co/rsk/federate/signing/hsm/config/PowHSMConfig.java b/src/main/java/co/rsk/federate/signing/hsm/config/PowHSMConfig.java index f5d9173c8..228cd89c6 100644 --- a/src/main/java/co/rsk/federate/signing/hsm/config/PowHSMConfig.java +++ b/src/main/java/co/rsk/federate/signing/hsm/config/PowHSMConfig.java @@ -5,6 +5,7 @@ import co.rsk.federate.signing.hsm.SignerException; import co.rsk.bitcoinj.core.NetworkParameters; import co.rsk.federate.signing.config.SignerConfig; +import co.rsk.federate.signing.config.SignerType; import co.rsk.federate.signing.hsm.HSMClientException; import co.rsk.federate.signing.hsm.HSMUnsupportedTypeException; import co.rsk.federate.signing.hsm.client.HSMBookkeepingClient; @@ -17,12 +18,11 @@ public class PowHSMConfig { private static final Logger logger = LoggerFactory.getLogger(PowHSMConfig.class); - private static final String SIGNER_TYPE = "hsm"; private final Config config; public PowHSMConfig(SignerConfig signerConfig) throws SignerException { - if (signerConfig == null || !signerConfig.getType().equals(SIGNER_TYPE)) { + if (signerConfig == null || signerConfig.getSignerType() != SignerType.HSM) { throw new SignerException("Signer config is not for PowHSM"); } diff --git a/src/test/java/co/rsk/federate/config/PowpegNodeSystemPropertiesTest.java b/src/test/java/co/rsk/federate/config/PowpegNodeSystemPropertiesTest.java index 164fb8bc2..fb07d2ada 100644 --- a/src/test/java/co/rsk/federate/config/PowpegNodeSystemPropertiesTest.java +++ b/src/test/java/co/rsk/federate/config/PowpegNodeSystemPropertiesTest.java @@ -303,6 +303,7 @@ void signerConfig_whenSignerConfigExists_shouldReturnSignerConfig() { when(mockSignersConfig.getObject(existingKey)).thenReturn(mockConfigSignerObject); when(mockConfigSignerObject.toConfig()).thenReturn(mockSignerConfig); when(mockSignerConfig.hasPath("type")).thenReturn(true); + when(mockSignerConfig.getString("type")).thenReturn("hsm"); SignerConfig result = powpegNodeSystemProperties.signerConfig(existingKey); diff --git a/src/test/java/co/rsk/federate/signing/ECDSASignerFactoryTest.java b/src/test/java/co/rsk/federate/signing/ECDSASignerFactoryTest.java index 162fd524a..aef094334 100644 --- a/src/test/java/co/rsk/federate/signing/ECDSASignerFactoryTest.java +++ b/src/test/java/co/rsk/federate/signing/ECDSASignerFactoryTest.java @@ -117,8 +117,8 @@ void buildFromConfigHSM() throws SignerException { @Test void buildFromConfigUnknown() throws SignerException { Config configMock = mockConfig("a-random-type"); - SignerConfig signerConfig = new SignerConfig("a-random-id", configMock); try { + SignerConfig signerConfig = new SignerConfig("a-random-id", configMock); factory.buildFromConfig(signerConfig); fail(); } catch (RuntimeException e) { diff --git a/src/test/java/co/rsk/federate/signing/hsm/config/PowHSMConfigTest.java b/src/test/java/co/rsk/federate/signing/hsm/config/PowHSMConfigTest.java index 67b6ddd95..621fcd3a3 100644 --- a/src/test/java/co/rsk/federate/signing/hsm/config/PowHSMConfigTest.java +++ b/src/test/java/co/rsk/federate/signing/hsm/config/PowHSMConfigTest.java @@ -11,6 +11,7 @@ import co.rsk.bitcoinj.core.NetworkParameters; import co.rsk.federate.signing.config.SignerConfig; +import co.rsk.federate.signing.config.SignerType; import co.rsk.federate.signing.hsm.HSMBlockchainBookkeepingRelatedException; import co.rsk.federate.signing.hsm.HSMClientException; import co.rsk.federate.signing.hsm.HSMUnsupportedTypeException; @@ -38,7 +39,7 @@ class PowHSMConfigTest { @BeforeEach void setup() throws Exception { when(signerConfig.getConfig()).thenReturn(config); - when(signerConfig.getType()).thenReturn("hsm"); + when(signerConfig.getSignerType()).thenReturn(SignerType.HSM); powHsmConfig = new PowHSMConfig(signerConfig); } From 662dc53fe9c00089331e1d5bb2bcc084c587f372 Mon Sep 17 00:00:00 2001 From: Marcos Date: Thu, 22 Aug 2024 12:38:16 -0300 Subject: [PATCH 3/7] Use equalsIgnoreCase in SignerType enum --- src/main/java/co/rsk/federate/signing/config/SignerType.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/co/rsk/federate/signing/config/SignerType.java b/src/main/java/co/rsk/federate/signing/config/SignerType.java index ec8b5cf8a..6e4473d7b 100644 --- a/src/main/java/co/rsk/federate/signing/config/SignerType.java +++ b/src/main/java/co/rsk/federate/signing/config/SignerType.java @@ -12,7 +12,7 @@ public enum SignerType { public static SignerType fromConfigValue(String configValue) { for (SignerType signerType : values()) { - if (signerType.getType().equals(configValue)) { + if (signerType.getType().equalsIgnoreCase(configValue)) { return signerType; } } From 72c4d7f26f26912371fd96bbff021eaccdb9d6ff Mon Sep 17 00:00:00 2001 From: Marcos Date: Thu, 22 Aug 2024 12:42:42 -0300 Subject: [PATCH 4/7] Use SignerType enum instead of string literals --- .../rsk/federate/config/PowpegNodeSystemPropertiesTest.java | 3 ++- .../java/co/rsk/federate/config/SignerConfigBuilder.java | 5 +++-- .../java/co/rsk/federate/signing/ECDSASignerFactoryTest.java | 5 +++-- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/test/java/co/rsk/federate/config/PowpegNodeSystemPropertiesTest.java b/src/test/java/co/rsk/federate/config/PowpegNodeSystemPropertiesTest.java index fb07d2ada..be880c4d8 100644 --- a/src/test/java/co/rsk/federate/config/PowpegNodeSystemPropertiesTest.java +++ b/src/test/java/co/rsk/federate/config/PowpegNodeSystemPropertiesTest.java @@ -10,6 +10,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import co.rsk.federate.signing.config.SignerType; import java.time.Duration; import java.util.ArrayList; import java.util.Arrays; @@ -303,7 +304,7 @@ void signerConfig_whenSignerConfigExists_shouldReturnSignerConfig() { when(mockSignersConfig.getObject(existingKey)).thenReturn(mockConfigSignerObject); when(mockConfigSignerObject.toConfig()).thenReturn(mockSignerConfig); when(mockSignerConfig.hasPath("type")).thenReturn(true); - when(mockSignerConfig.getString("type")).thenReturn("hsm"); + when(mockSignerConfig.getString("type")).thenReturn(SignerType.HSM.getType()); SignerConfig result = powpegNodeSystemProperties.signerConfig(existingKey); diff --git a/src/test/java/co/rsk/federate/config/SignerConfigBuilder.java b/src/test/java/co/rsk/federate/config/SignerConfigBuilder.java index 0b66dc199..e4ca99616 100644 --- a/src/test/java/co/rsk/federate/config/SignerConfigBuilder.java +++ b/src/test/java/co/rsk/federate/config/SignerConfigBuilder.java @@ -2,6 +2,7 @@ import static com.typesafe.config.ConfigValueFactory.fromAnyRef; +import co.rsk.federate.signing.config.SignerType; import java.math.BigInteger; import com.typesafe.config.Config; import com.typesafe.config.ConfigFactory; @@ -25,7 +26,7 @@ public SignerConfigBuilder withValue(String path, Object value) { } public SignerConfigBuilder withHsmSigner(String keyId) { - this.config = this.config.withValue("type", fromAnyRef("hsm")); + this.config = this.config.withValue("type", fromAnyRef(SignerType.HSM.getType())); this.config = this.config.withValue("host", fromAnyRef("127.0.0.1")); this.config = this.config.withValue("port", fromAnyRef(9999)); this.config = this.config.withValue("keyId", fromAnyRef(keyId)); @@ -48,7 +49,7 @@ public SignerConfigBuilder withHsmBookkeepingInfo(BigInteger difficultyTarget, l } public SignerConfigBuilder withKeyFileSigner(String path) { - this.config = this.config.withValue("type", fromAnyRef("keyFile")); + this.config = this.config.withValue("type", fromAnyRef(SignerType.KEYFILE.getType())); this.config = this.config.withValue("path", fromAnyRef(path)); return this; } diff --git a/src/test/java/co/rsk/federate/signing/ECDSASignerFactoryTest.java b/src/test/java/co/rsk/federate/signing/ECDSASignerFactoryTest.java index aef094334..25711c757 100644 --- a/src/test/java/co/rsk/federate/signing/ECDSASignerFactoryTest.java +++ b/src/test/java/co/rsk/federate/signing/ECDSASignerFactoryTest.java @@ -28,6 +28,7 @@ import co.rsk.federate.signing.config.SignerConfig; import co.rsk.federate.rpc.JsonRpcClientProvider; import co.rsk.federate.rpc.SocketBasedJsonRpcClientProvider; +import co.rsk.federate.signing.config.SignerType; import co.rsk.federate.signing.hsm.SignerException; import co.rsk.federate.signing.hsm.client.HSMClientProtocol; import co.rsk.federate.signing.hsm.client.HSMSigningClientProvider; @@ -49,7 +50,7 @@ void createFactory() { @Test void buildFromConfigKeyFile() throws SignerException { - Config configMock = mockConfig("keyFile"); + Config configMock = mockConfig(SignerType.KEYFILE.getType()); when(configMock.getString("path")).thenReturn("a-random-path"); SignerConfig signerConfig = new SignerConfig("a-random-id", configMock); ECDSASigner signer = factory.buildFromConfig(signerConfig); @@ -61,7 +62,7 @@ void buildFromConfigKeyFile() throws SignerException { @Test void buildFromConfigHSM() throws SignerException { - Config configMock = mockConfig("hsm"); + Config configMock = mockConfig(SignerType.HSM.getType()); when(configMock.hasPath("host")).thenReturn(true); when(configMock.getString("host")).thenReturn("remotehost"); when(configMock.hasPath("port")).thenReturn(true); From 97d58c30027beb0635a41a58bd9cdc9d44506ada Mon Sep 17 00:00:00 2001 From: Marcos Date: Thu, 22 Aug 2024 13:05:47 -0300 Subject: [PATCH 5/7] Change exception thrown in SignerType --- src/main/java/co/rsk/federate/signing/config/SignerType.java | 2 +- .../java/co/rsk/federate/signing/ECDSASignerFactoryTest.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/co/rsk/federate/signing/config/SignerType.java b/src/main/java/co/rsk/federate/signing/config/SignerType.java index 6e4473d7b..295b74e0d 100644 --- a/src/main/java/co/rsk/federate/signing/config/SignerType.java +++ b/src/main/java/co/rsk/federate/signing/config/SignerType.java @@ -16,7 +16,7 @@ public static SignerType fromConfigValue(String configValue) { return signerType; } } - throw new RuntimeException(String.format("Unsupported signer type: %s", configValue)); + throw new IllegalArgumentException(String.format("Unsupported signer type: %s", configValue)); } public String getType() { diff --git a/src/test/java/co/rsk/federate/signing/ECDSASignerFactoryTest.java b/src/test/java/co/rsk/federate/signing/ECDSASignerFactoryTest.java index 25711c757..23bace851 100644 --- a/src/test/java/co/rsk/federate/signing/ECDSASignerFactoryTest.java +++ b/src/test/java/co/rsk/federate/signing/ECDSASignerFactoryTest.java @@ -118,8 +118,8 @@ void buildFromConfigHSM() throws SignerException { @Test void buildFromConfigUnknown() throws SignerException { Config configMock = mockConfig("a-random-type"); + SignerConfig signerConfig = new SignerConfig("a-random-id", configMock); try { - SignerConfig signerConfig = new SignerConfig("a-random-id", configMock); factory.buildFromConfig(signerConfig); fail(); } catch (RuntimeException e) { From 6e12eca9f2350c2c46f4724c774d67f6395eac8e Mon Sep 17 00:00:00 2001 From: Marcos Date: Thu, 22 Aug 2024 13:20:50 -0300 Subject: [PATCH 6/7] Refactor ECDSASignerFactory to encapsulate logic in methods --- .../java/co/rsk/federate/FedNodeRunner.java | 56 ++++++++++--------- .../federate/signing/ECDSASignerFactory.java | 39 +++++++------ 2 files changed, 50 insertions(+), 45 deletions(-) diff --git a/src/main/java/co/rsk/federate/FedNodeRunner.java b/src/main/java/co/rsk/federate/FedNodeRunner.java index 233bc4ed5..b5f876e8f 100644 --- a/src/main/java/co/rsk/federate/FedNodeRunner.java +++ b/src/main/java/co/rsk/federate/FedNodeRunner.java @@ -21,6 +21,7 @@ import co.rsk.NodeRunner; import co.rsk.bitcoinj.core.BtcECKey; +import co.rsk.core.RskAddress; import co.rsk.federate.signing.hsm.HSMVersion; import co.rsk.peg.constants.BridgeConstants; import co.rsk.federate.adapter.ThinConverter; @@ -73,7 +74,7 @@ */ public class FedNodeRunner implements NodeRunner { - private static final Logger LOGGER = LoggerFactory.getLogger(FedNodeRunner.class); + private static final Logger logger = LoggerFactory.getLogger(FedNodeRunner.class); private final BtcToRskClient btcToRskClientActive; private final BtcToRskClient btcToRskClientRetiring; private final BtcReleaseClient btcReleaseClient; @@ -126,7 +127,7 @@ public FedNodeRunner( @Override public void run() throws Exception { - LOGGER.debug("[run] Starting RSK"); + logger.debug("[run] Starting RSK"); signer = buildSigner(); SignerConfig btcSignerConfig = config.signerConfig(BTC.getId()); @@ -135,22 +136,23 @@ public void run() throws Exception { } if (!this.checkFederateRequirements()) { - LOGGER.error("[run] Error validating Fed-Node Requirements"); + logger.error("[run] Error validating Fed-Node Requirements"); return; } - LOGGER.info("[run] Signers: {}", signer.getVersionString()); + logger.info("[run] Signers: {}", signer.getVersionString()); configureFederatorSupport(); fullNodeRunner.run(); startFederate(); signer.addListener(l -> { - LOGGER.error("[run] Signer informed unrecoverable state, shutting down", l); + logger.error("[run] Signer informed unrecoverable state, shutting down", l); this.shutdown(); }); - LOGGER.info("[run] Federated node started"); - LOGGER.info("[run] RSK address: {}", Hex.toHexString(this.member.getRskPublicKey().getAddress())); + RskAddress pegnatoryRskAddress = new RskAddress(this.member.getRskPublicKey().getAddress()); + logger.info("[run] Federated node started"); + logger.info("[run] RSK address: {}", pegnatoryRskAddress); } private void startBookkeepingServices() throws SignerException, HSMClientException { @@ -161,7 +163,7 @@ private void startBookkeepingServices() throws SignerException, HSMClientExcepti hsmClientProtocolFactory.buildHSMClientProtocolFromConfig(powHsmConfig); int hsmVersion = protocol.getVersion(); - LOGGER.debug("[run] Using HSM version {}", hsmVersion); + logger.debug("[run] Using HSM version {}", hsmVersion); if (HSMVersion.isPowHSM(hsmVersion)) { hsmBookkeepingClient = buildBookKeepingClient( @@ -175,7 +177,7 @@ private void configureFederatorSupport() throws SignerException { BtcECKey btcPublicKey = signer.getPublicKey(BTC.getKeyId()).toBtcKey(); ECKey rskPublicKey = signer.getPublicKey(RSK.getKeyId()).toEthKey(); ECKey mstKey = signer.getPublicKey(MST.getKeyId()).toEthKey(); - LOGGER.info( + logger.info( "[configureFederatorSupport] BTC public key: {}. RSK public key: {}. MST public key: {}", btcPublicKey, rskPublicKey, @@ -200,14 +202,14 @@ private ECDSASigner buildSigner() { ECDSASigner createdSigner = buildSignerFromKey(keyId.getKeyId()); compositeSigner.addSigner(createdSigner); } catch (SignerException e) { - LOGGER.error("[buildSigner] Error trying to build signer with key id {}. Detail: {}", keyId, e.getMessage()); + logger.error("[buildSigner] Error trying to build signer with key id {}. Detail: {}", keyId, e.getMessage()); } catch (Exception e) { - LOGGER.error("[buildSigner] Error creating signer {}. Detail: {}", keyId, e.getMessage()); + logger.error("[buildSigner] Error creating signer {}. Detail: {}", keyId, e.getMessage()); throw e; } }); - LOGGER.debug("[buildSigner] Signers created"); + logger.debug("[buildSigner] Signers created"); return compositeSigner; } @@ -218,7 +220,7 @@ private HSMBookkeepingClient buildBookKeepingClient( HSMBookkeepingClient bookKeepingClient = hsmBookKeepingClientProvider.getHSMBookKeepingClient(protocol); bookKeepingClient.setMaxChunkSizeToHsm(powHsmConfig.getMaxChunkSizeToHsm()); - LOGGER.info("[buildBookKeepingClient] HSMBookkeeping client built for HSM version: {}", bookKeepingClient.getVersion()); + logger.info("[buildBookKeepingClient] HSMBookkeeping client built for HSM version: {}", bookKeepingClient.getVersion()); return bookKeepingClient; } @@ -240,7 +242,7 @@ private HSMBookkeepingService buildBookKeepingService( powHsmConfig.getInformerInterval(), powHsmConfig.isStopBookkeepingScheduler() ); - LOGGER.info("[buildBookKeepingService] HSMBookkeeping Service built for HSM version: {}", bookKeepingClient.getVersion()); + logger.info("[buildBookKeepingService] HSMBookkeeping Service built for HSM version: {}", bookKeepingClient.getVersion()); return service; } @@ -274,7 +276,7 @@ private boolean checkFederateRequirements() { } private void startFederate() throws Exception { - LOGGER.debug("[startFederate] Starting Federation Behaviour"); + logger.debug("[startFederate] Starting Federation Behaviour"); if (config.isFederatorEnabled()) { // Set up a federation watcher to trigger starts and stops of the // btc to rsk client upon federation changes @@ -308,8 +310,8 @@ private void startFederate() throws Exception { btcLogMonitor.start(); rskLogMonitor.start(); if (hsmBookkeepingService != null) { - hsmBookkeepingService.addListener((e) -> { - LOGGER.error("[startFederate] HSM bookkeeping service informed unrecoverable state, shutting down", e); + hsmBookkeepingService.addListener(e -> { + logger.error("[startFederate] HSM bookkeeping service informed unrecoverable state, shutting down", e); this.shutdown(); }); hsmBookkeepingService.start(); @@ -348,7 +350,7 @@ private void startFederate() throws Exception { public void onActiveFederationChange(Optional oldFederation, Federation newFederation) { String oldFederationAddress = oldFederation.map(f -> f.getAddress().toString()).orElse("NONE"); String newFederationAddress = newFederation.getAddress().toString(); - LOGGER.debug(String.format("[onActiveFederationChange] Active federation change: from %s to %s", oldFederationAddress, newFederationAddress)); + logger.debug(String.format("[onActiveFederationChange] Active federation change: from %s to %s", oldFederationAddress, newFederationAddress)); triggerClientChange(btcToRskClientActive, Optional.of(newFederation)); } @@ -356,7 +358,7 @@ public void onActiveFederationChange(Optional oldFederation, Federat public void onRetiringFederationChange(Optional oldFederation, Optional newFederation) { String oldFederationAddress = oldFederation.map(f -> f.getAddress().toString()).orElse("NONE"); String newFederationAddress = newFederation.map(f -> f.getAddress().toString()).orElse("NONE"); - LOGGER.debug(String.format("[onRetiringFederationChange] Retiring federation change: from %s to %s", oldFederationAddress, newFederationAddress)); + logger.debug(String.format("[onRetiringFederationChange] Retiring federation change: from %s to %s", oldFederationAddress, newFederationAddress)); triggerClientChange(btcToRskClientRetiring, newFederation); } }); @@ -367,18 +369,18 @@ public void onRetiringFederationChange(Optional oldFederation, Optio @PreDestroy public void tearDown() { - LOGGER.debug("[tearDown] FederateRunner tearDown starting..."); + logger.debug("[tearDown] FederateRunner tearDown starting..."); this.stop(); - LOGGER.debug("[tearDown] FederateRunner tearDown finished."); + logger.debug("[tearDown] FederateRunner tearDown finished."); } private void shutdown() { try { this.tearDown(); } catch(Exception e){ - LOGGER.error("[shutdown] FederateRunner teardown failed", e); + logger.error("[shutdown] FederateRunner teardown failed", e); } System.exit(-1); } @@ -390,18 +392,18 @@ private void triggerClientChange(BtcToRskClient client, Optional fed // Only start if this federator is part of the new federation if (federation.isPresent() && federation.get().isMember(this.member)) { String federationAddress = federation.get().getAddress().toString(); - LOGGER.debug("[triggerClientChange] Starting lock and release clients since I belong to federation {}", federationAddress); - LOGGER.info("[triggerClientChange] Joined to {} federation", federationAddress); + logger.debug("[triggerClientChange] Starting lock and release clients since I belong to federation {}", federationAddress); + logger.info("[triggerClientChange] Joined to {} federation", federationAddress); client.start(federation.get()); btcReleaseClient.start(federation.get()); } else { - LOGGER.warn("[triggerClientChange] This federator node is not part of the new federation. Check your configuration for signers BTC, RSK and MST keys"); + logger.warn("[triggerClientChange] This federator node is not part of the new federation. Check your configuration for signers BTC, RSK and MST keys"); } } @Override public void stop() { - LOGGER.info("[stop] Shutting down Federation node"); + logger.info("[stop] Shutting down Federation node"); if (bitcoinWrapper != null) { bitcoinWrapper.stop(); } @@ -417,7 +419,7 @@ public void stop() { } fullNodeRunner.stop(); - LOGGER.info("[stop] Federation node Shut down."); + logger.info("[stop] Federation node Shut down."); } private BitcoinWrapper createAndSetupBitcoinWrapper( diff --git a/src/main/java/co/rsk/federate/signing/ECDSASignerFactory.java b/src/main/java/co/rsk/federate/signing/ECDSASignerFactory.java index 2a4503b06..45348be62 100644 --- a/src/main/java/co/rsk/federate/signing/ECDSASignerFactory.java +++ b/src/main/java/co/rsk/federate/signing/ECDSASignerFactory.java @@ -40,27 +40,30 @@ public ECDSASigner buildFromConfig(SignerConfig config) throws SignerException { switch (type) { case KEYFILE: return new ECDSASignerFromFileKey( - new KeyId(config.getId()), - config.getConfig().getString("path") + new KeyId(config.getId()), + config.getConfig().getString("path") ); case HSM: - try { - PowHSMConfig powHSMConfig = new PowHSMConfig(config); - HSMClientProtocol hsmClientProtocol = - new HSMClientProtocolFactory().buildHSMClientProtocolFromConfig(powHSMConfig); - HSMSigningClientProvider hsmSigningClientProvider = new HSMSigningClientProvider(hsmClientProtocol, config.getId()); - ECDSAHSMSigner signer = new ECDSAHSMSigner(hsmSigningClientProvider); - // Add the key mapping - String hsmKeyId = config.getConfig().getString("keyId"); - signer.addKeyMapping(new KeyId(config.getId()), hsmKeyId); - return signer; - } catch (Exception e) { - String message = "Something went wrong while trying to build HSM Signer"; - logger.debug("[buildFromConfig] {} - {}", message, e.getMessage()); - throw new RuntimeException(e.getMessage()); - } + return buildHSMFromConfig(config); default: - throw new RuntimeException(String.format("Unsupported signer type: %s", type)); + throw new IllegalArgumentException(String.format("Unsupported signer type: %s", type)); } } + + private ECDSAHSMSigner buildHSMFromConfig(SignerConfig config) throws SignerException { + PowHSMConfig powHSMConfig = new PowHSMConfig(config); + HSMClientProtocol hsmClientProtocol = new HSMClientProtocolFactory().buildHSMClientProtocolFromConfig( + powHSMConfig + ); + HSMSigningClientProvider hsmSigningClientProvider = new HSMSigningClientProvider( + hsmClientProtocol, + config.getId() + ); + ECDSAHSMSigner signer = new ECDSAHSMSigner(hsmSigningClientProvider); + // Add the key mapping + String hsmKeyId = config.getConfig().getString("keyId"); + signer.addKeyMapping(new KeyId(config.getId()), hsmKeyId); + + return signer; + } } From 413fd23043c9220ed5d25b918b037b08ab405bfe Mon Sep 17 00:00:00 2001 From: Marcos Date: Thu, 22 Aug 2024 13:28:31 -0300 Subject: [PATCH 7/7] Update test --- .../co/rsk/federate/signing/ECDSASignerFactoryTest.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/test/java/co/rsk/federate/signing/ECDSASignerFactoryTest.java b/src/test/java/co/rsk/federate/signing/ECDSASignerFactoryTest.java index 23bace851..154df7715 100644 --- a/src/test/java/co/rsk/federate/signing/ECDSASignerFactoryTest.java +++ b/src/test/java/co/rsk/federate/signing/ECDSASignerFactoryTest.java @@ -116,13 +116,12 @@ void buildFromConfigHSM() throws SignerException { } @Test - void buildFromConfigUnknown() throws SignerException { + void buildFromConfigUnknown() { Config configMock = mockConfig("a-random-type"); - SignerConfig signerConfig = new SignerConfig("a-random-id", configMock); try { - factory.buildFromConfig(signerConfig); + new SignerConfig("a-random-id", configMock); fail(); - } catch (RuntimeException e) { + } catch (IllegalArgumentException e) { assertEquals("Unsupported signer type: a-random-type", e.getMessage()); } }