Skip to content

Commit

Permalink
cleaning SignerForIdentifier generic usage
Browse files Browse the repository at this point in the history
  • Loading branch information
usmansaleem committed Nov 7, 2024
1 parent ad2e0ef commit 30c6f1b
Show file tree
Hide file tree
Showing 17 changed files with 43 additions and 70 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
import org.apache.tuweni.bytes.Bytes32;

public class CommitBoostGenerateProxyKeyHandler implements Handler<RoutingContext> {
private static final Logger LOG = LogManager.getLogger();
private static final ObjectMapper JSON_MAPPER = SigningObjectMapperFactory.createObjectMapper();
private static final int NOT_FOUND = 404;
private static final int BAD_REQUEST = 400;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,9 @@
package tech.pegasys.web3signer.core.service.http.handlers.commitboost;

import tech.pegasys.web3signer.core.service.http.handlers.commitboost.json.SignRequestType;
import tech.pegasys.web3signer.signing.ArtifactSignature;
import tech.pegasys.web3signer.signing.ArtifactSigner;
import tech.pegasys.web3signer.signing.ArtifactSignerProvider;
import tech.pegasys.web3signer.signing.BlsArtifactSignature;
import tech.pegasys.web3signer.signing.KeyType;
import tech.pegasys.web3signer.signing.SecpArtifactSignature;

import java.util.Map;
import java.util.Optional;
Expand Down Expand Up @@ -83,21 +80,7 @@ public Optional<String> sign(
? artifactSignerProvider.getSigner(identifier)
: artifactSignerProvider.getProxySigner(identifier);

return optionalArtifactSigner
.map(
signer -> {
final ArtifactSignature artifactSignature = signer.sign(signingRoot);
return switch (artifactSignature.getType()) {
case BLS ->
Optional.of(
((BlsArtifactSignature) artifactSignature).getSignatureData().toString());
case SECP256K1 ->
Optional.of(
SecpArtifactSignature.toBytes((SecpArtifactSignature) artifactSignature)
.toHexString());
};
})
.orElse(Optional.empty());
return optionalArtifactSigner.map(signer -> signer.sign(signingRoot).asHex());
}

public void addProxySigner(final ArtifactSigner artifactSigner, final String identifier) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,11 @@
public class Eth1SignForIdentifierHandler implements Handler<RoutingContext> {

private static final Logger LOG = LogManager.getLogger();
private final SignerForIdentifier<?> signerForIdentifier;
private final SignerForIdentifier signerForIdentifier;
private final HttpApiMetrics metrics;

public Eth1SignForIdentifierHandler(
final SignerForIdentifier<?> signerForIdentifier, final HttpApiMetrics metrics) {
final SignerForIdentifier signerForIdentifier, final HttpApiMetrics metrics) {
this.signerForIdentifier = signerForIdentifier;
this.metrics = metrics;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,9 @@ public class SigningExtensionHandler implements Handler<RoutingContext> {
.copy()
.enable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES);

private final SignerForIdentifier<?> signerForIdentifier;
private final SignerForIdentifier signerForIdentifier;

public SigningExtensionHandler(final SignerForIdentifier<?> signerForIdentifier) {
public SigningExtensionHandler(final SignerForIdentifier signerForIdentifier) {
this.signerForIdentifier = signerForIdentifier;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@
public class Eth2SignForIdentifierHandler implements Handler<RoutingContext> {

private static final Logger LOG = LogManager.getLogger();
private final SignerForIdentifier<?> signerForIdentifier;
private final SignerForIdentifier signerForIdentifier;
private final HttpApiMetrics httpMetrics;
private final SlashingProtectionMetrics slashingMetrics;
private final Optional<SlashingProtection> slashingProtection;
Expand All @@ -69,7 +69,7 @@ public class Eth2SignForIdentifierHandler implements Handler<RoutingContext> {
public static final int SLASHING_PROTECTION_ENFORCED = 412;

public Eth2SignForIdentifierHandler(
final SignerForIdentifier<?> signerForIdentifier,
final SignerForIdentifier signerForIdentifier,
final HttpApiMetrics httpMetrics,
final SlashingProtectionMetrics slashingMetrics,
final Optional<SlashingProtection> slashingProtection,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,10 @@ public class EthSignResultProvider implements ResultProvider<String> {

private static final Logger LOG = LogManager.getLogger();

private final SignerForIdentifier<SecpArtifactSignature> transactionSignerProvider;
private final SignerForIdentifier transactionSignerProvider;

public EthSignResultProvider(
final SignerForIdentifier<SecpArtifactSignature> transactionSignerProvider) {
final SignerForIdentifier transactionSignerProvider) {
this.transactionSignerProvider = transactionSignerProvider;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,11 @@ public class EthSignTransactionResultProvider implements ResultProvider<String>

private final long chainId;
private final JsonDecoder decoder;
private final SignerForIdentifier<SecpArtifactSignature> secpSigner;
private final SignerForIdentifier secpSigner;

public EthSignTransactionResultProvider(
final long chainId,
final SignerForIdentifier<SecpArtifactSignature> secpSigner,
final SignerForIdentifier secpSigner,
final JsonDecoder decoder) {
this.chainId = chainId;
this.decoder = decoder;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,10 @@ public class EthSignTypedDataResultProvider implements ResultProvider<String> {

private static final Logger LOG = LogManager.getLogger();

private final SignerForIdentifier<SecpArtifactSignature> transactionSignerProvider;
private final SignerForIdentifier transactionSignerProvider;

public EthSignTypedDataResultProvider(
final SignerForIdentifier<SecpArtifactSignature> transactionSignerProvider) {
final SignerForIdentifier transactionSignerProvider) {
this.transactionSignerProvider = transactionSignerProvider;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,14 @@ public class SendTransactionHandler implements JsonRpcRequestHandler {
private final TransactionFactory transactionFactory;
private final VertxRequestTransmitterFactory vertxTransmitterFactory;

private final SignerForIdentifier<SecpArtifactSignature> secpSigner;
private final SignerForIdentifier secpSigner;
private static final int MAX_NONCE_RETRIES = 10;

public SendTransactionHandler(
final long chainId,
final TransactionFactory transactionFactory,
final VertxRequestTransmitterFactory vertxTransmitterFactory,
final SignerForIdentifier<SecpArtifactSignature> secpSigner) {
final SignerForIdentifier secpSigner) {
this.chainId = chainId;
this.transactionFactory = transactionFactory;
this.vertxTransmitterFactory = vertxTransmitterFactory;
Expand Down Expand Up @@ -89,7 +89,7 @@ public void handle(final RoutingContext context, final JsonRpcRequest request) {
private void sendTransaction(
final Transaction transaction,
final RoutingContext routingContext,
final SignerForIdentifier<SecpArtifactSignature> secpSigner,
final SignerForIdentifier secpSigner,
final JsonRpcRequest request) {

final TransactionSerializer transactionSerializer =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,10 @@

public class TransactionSerializer {

protected final SignerForIdentifier<SecpArtifactSignature> secpSigner;
protected final SignerForIdentifier secpSigner;
protected final long chainId;

public TransactionSerializer(
final SignerForIdentifier<SecpArtifactSignature> secpSigner, final long chainId) {
public TransactionSerializer(final SignerForIdentifier secpSigner, final long chainId) {
this.secpSigner = secpSigner;
this.chainId = chainId;
}
Expand Down Expand Up @@ -79,9 +78,11 @@ private static byte[] prependEip1559TransactionType(byte[] bytesToSign) {

private SignatureData sign(final String eth1Address, final byte[] bytesToSign) {
final SecpArtifactSignature artifactSignature =
secpSigner
.signAndGetArtifactSignature(normaliseIdentifier(eth1Address), Bytes.of(bytesToSign))
.orElseThrow(() -> new JsonRpcException(SIGNING_FROM_IS_NOT_AN_UNLOCKED_ACCOUNT));
(SecpArtifactSignature)
secpSigner
.signAndGetArtifactSignature(
normaliseIdentifier(eth1Address), Bytes.of(bytesToSign))
.orElseThrow(() -> new JsonRpcException(SIGNING_FROM_IS_NOT_AN_UNLOCKED_ACCOUNT));

final Signature signature = artifactSignature.getSignatureData();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ void computeSigningRootForBLSProxyKey(final Eth2Network network) {

// verify BLS Signature matching Commit Boost client implementation as well
final BlsArtifactSigner artifactSigner = new BlsArtifactSigner(DELEGATOR_KEY_PAIR, null);
final String signature = artifactSigner.sign(signingRoot).getSignatureData().toString();
final String signature = artifactSigner.sign(signingRoot).asHex();

assertThat(signature).isEqualTo(BLS_PROXY_MESSAGE_SIGNATURE_MAP.get(network));
}
Expand All @@ -178,7 +178,7 @@ void computeSigningRootforSECPProxyKey(final Eth2Network network) {
// verify BLS Signature matching Commit Boost client implementation as well
final BlsArtifactSigner artifactSigner = new BlsArtifactSigner(DELEGATOR_KEY_PAIR, null);
BlsArtifactSignature blsArtifactSignature = artifactSigner.sign(signingRoot);
String signature = blsArtifactSignature.getSignatureData().toString();
String signature = blsArtifactSignature.asHex();

assertThat(signature).isEqualTo(SECP_PROXY_MESSAGE_SIGNATURE_MAP.get(network));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@
@ExtendWith(MockitoExtension.class)
public class EthSignResultProviderTest {

@Mock SignerForIdentifier<SecpArtifactSignature> transactionSignerProvider;
@Mock SignerForIdentifier transactionSignerProvider;

@ParameterizedTest
@ArgumentsSource(InvalidParamsProvider.class)
Expand Down Expand Up @@ -97,7 +97,7 @@ public void signatureHasTheExpectedFormat() {
final SecpArtifactSignature secpArtifactSignature =
new SecpArtifactSignature(new Signature(v, r, s));

doReturn(Optional.of(SecpArtifactSignature.toBytes(secpArtifactSignature).toHexString()))
doReturn(Optional.of(secpArtifactSignature.asHex()))
.when(transactionSignerProvider)
.sign(any(), any(Bytes.class));

Expand Down Expand Up @@ -138,13 +138,12 @@ public void returnsExpectedSignature(final String message) {
Bytes data = answer.getArgument(1, Bytes.class);
final Sign.SignatureData signature = Sign.signMessage(data.toArrayUnsafe(), keyPair);
return Optional.of(
SecpArtifactSignature.toBytes(
new SecpArtifactSignature(
new Signature(
new BigInteger(signature.getV()),
new BigInteger(1, signature.getR()),
new BigInteger(1, signature.getS()))))
.toHexString());
new SecpArtifactSignature(
new Signature(
new BigInteger(signature.getV()),
new BigInteger(1, signature.getR()),
new BigInteger(1, signature.getS())))
.asHex());
})
.when(transactionSignerProvider)
.sign(anyString(), any(Bytes.class));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ public class EthSignTypedDataResultProviderTest {

private static final ECKeyPair KEY_PAIR = new ECKeyPair(PRIVATE_KEY, PUBLIC_KEY);

@Mock SignerForIdentifier<SecpArtifactSignature> transactionSignerProvider;
@Mock SignerForIdentifier transactionSignerProvider;

@ParameterizedTest
@ArgumentsSource(InvalidParamsProvider.class)
Expand Down Expand Up @@ -147,12 +147,11 @@ public Stream<? extends Arguments> provideArguments(final ExtensionContext conte
}

private String hexFromSignatureData(Sign.SignatureData signature) {
return SecpArtifactSignature.toBytes(
new SecpArtifactSignature(
new Signature(
new BigInteger(signature.getV()),
new BigInteger(1, signature.getR()),
new BigInteger(1, signature.getS()))))
.toHexString();
return new SecpArtifactSignature(
new Signature(
new BigInteger(signature.getV()),
new BigInteger(1, signature.getR()),
new BigInteger(1, signature.getS())))
.asHex();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ public BLSSignature getSignatureData() {
return blsSignature;
}

@Override
public String asHex() {
return blsSignature.toString();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,13 +54,4 @@ public static SecpArtifactSignature fromBytes(final Bytes signature) {
Numeric.toBigInt(r.toArrayUnsafe()),
Numeric.toBigInt(s.toArrayUnsafe())));
}

@Deprecated // use asHex instead
public static Bytes toBytes(final SecpArtifactSignature signature) {
final Signature signatureData = signature.getSignatureData();
return Bytes.concatenate(
Bytes32.leftPad(Bytes.wrap(ByteUtils.bigIntegerToBytes(signatureData.getR()))),
Bytes32.leftPad(Bytes.wrap(ByteUtils.bigIntegerToBytes(signatureData.getS()))),
Bytes.wrap(ByteUtils.bigIntegerToBytes(signatureData.getV())));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ void hexEncodedSignatureIsReturned() {
final BLSSignature blsSignature =
BLSSignature.fromBytesCompressed(Bytes.fromHexString(SIGNATURE));
final BlsArtifactSignature blsArtifactSignature = new BlsArtifactSignature(blsSignature);
assertThat(blsArtifactSignature.getSignatureData().toString()).isEqualTo(SIGNATURE);
assertThat(blsArtifactSignature.asHex()).isEqualTo(SIGNATURE);
assertThat(blsSignature.toBytesCompressed().toHexString()).isEqualTo(SIGNATURE);
assertThat(blsSignature.toString()).isEqualTo(SIGNATURE);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,6 @@ void signsData() {
new BlsArtifactSigner(keyPair, SignerOrigin.FILE_RAW);
final BlsArtifactSignature signature = blsArtifactSigner.sign(message);

assertThat(signature.getSignatureData().toString()).isEqualTo(expectedSignature.toString());
assertThat(signature.asHex()).isEqualTo(expectedSignature.toString());
}
}

0 comments on commit 30c6f1b

Please sign in to comment.