Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/364 integretion with hsm #1602

Conversation

andrii-kl
Copy link
Contributor

What does this PR do?

Add functionality to work with HSM (hardware security module). Reference implementation of working with HSM over HTTP

Why is it needed?

To sign transactions with hard wallet / HSM (hardware security module)

@andrii-kl andrii-kl requested a review from AlexandrouR January 17, 2022 10:55
@andrii-kl andrii-kl self-assigned this Jan 17, 2022
@andrii-kl andrii-kl requested a review from conor10 as a code owner January 17, 2022 10:55
@andrii-kl andrii-kl linked an issue Jan 18, 2022 that may be closed by this pull request
Copy link
Contributor

@AlexandrouR AlexandrouR left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@AlexandrouR AlexandrouR merged commit 5dd98c2 into hyperledger-web3j:master Jan 20, 2022
@andrii-kl andrii-kl linked an issue Feb 16, 2022 that may be closed by this pull request
} else {
signedMessage = PrivateTransactionEncoder.signMessage(rawTransaction, credentials);
}
final byte[] signedMessage = txSignService.sign(rawTransaction, chainId);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @andrii-kl , I think this change is not ok since it gets rid of the PrivateTransactionEncoder to replace it with the regular TransactionEncoder within TxSignServiceImpl .

I have run some tests in Besu using 4.9.3 and indeed the signature is different.

Let me know if I am missing something, otherwise probably it would be good to open an issue for the fix.

Thanks in advance, Miguel.

Copy link
Contributor

@antonydenyer antonydenyer Jul 12, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just spotted this also! #1747

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for creating the issue @antonydenyer ! If @andrii-kl is not available I could take the implementation of the fix.

@nissimr
Copy link

nissimr commented Feb 19, 2023

Is there any documentation about how to use this with a hardware wallet?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Integration with KMS Adding support of hardware wallets to web3j
5 participants