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

Add Ethereum Support for HD Keys #652

Merged
merged 1 commit into from
Nov 25, 2019

Conversation

shemnon
Copy link
Contributor

@shemnon shemnon commented Nov 14, 2019

This PR adds support for each Caliper client to have its own Ethereum
account to be deterministically derived from a seed value using BIP44 style
key derivation. This is only supported for the fromAddress value.

This enables besu support of multiple caliper clients instead of single
client operations.

To use this instead of specifying a fromAddresss and an optional private
key instead a fromAddressSeed value is specified, which is a 128 bit hex
encoded value used as the seed for key derivation. Each client uses its
client index as the account field in the BIP-44 path. In the future where multiple
accounts are needed per caliper client the address_index field will be
incremented or mapped to the account value.

The genesis file in the samples and integration tests for besu have had
the first 10 keys pre-seeded with test ether. The sample and
integration test for besu has also been updated to use a from address
seed.

Signed-off-by: Danno Ferrin [email protected]

Checklist

  • A link to the issue/user story that the pull request relates to
  • How to recreate the problem without the fix
  • Design of the fix
  • How to prove that the fix works
  • Automated tests that prove the fix keeps on working
  • Documentation - any JSDoc, website, or Stackoverflow answers?

Issue/User story

Steps to Reproduce

Existing issues

Design of the fix

Validation of the fix

Automated Tests

What documentation has been provided for this pull request

@nklincoln nklincoln self-assigned this Nov 19, 2019
@nklincoln nklincoln added the PR/under review The PR is currently under review label Nov 19, 2019
Copy link
Contributor

@nklincoln nklincoln left a comment

Choose a reason for hiding this comment

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

Hi,
Thanks for the PR, the changes look good but I do have a request for the changes to the final integration test. Ideally we should be capturing the change in a test, and the ideal one here would be to do similar to what is done in the Fabric CI suite where multiple tests are run with different configuration files. In this case it is possible to run a test using the original configuration file, and then another with the config file that specifies the fromAddressSeed

The new configuration option should also be added to the docs

@shemnon
Copy link
Contributor Author

shemnon commented Nov 20, 2019

So one config file per feature in the integration? The integration test prior to the latest reboot had both explicit gas and estimated gas tests, I can set that up in separate tests, so I see 3 - estimated gas, explicit gas, and multi client HD wallets.

@aklenik
Copy link
Contributor

aklenik commented Nov 20, 2019

@shemnon Yeah, something like that. The Fabric tests use the same network for every phase (start network, perform different stuffs different ways, shutdown network). I don't know whether this is feasible for the Ethereum/Besu tests. Should be, even if you deploy the same contract multiple times on different addresses.

account to be deterministically derived from a seed value using BIP44 style
key derivation. This is only supported for the fromAddress value.

This enables besu support of multiple caliper clients instead of single
client operations.

The genesis file in the integration tests for besu have had the first 10
keys pre-seeded with test ether. The integration test for besu has also
been updated to use a from address seed. The integration test is now a
multi stage test with single client in stage 2 and multi client in stage
3.

Signed-off-by: Danno Ferrin <[email protected]>
@shemnon
Copy link
Contributor Author

shemnon commented Nov 22, 2019

Integration tests updated (right now it's single client then dual client, we get the coverage we need). Docs are updated in #666 and samples are updated in hyperledger-caliper/caliper-benchmarks#33

Copy link
Contributor

@nklincoln nklincoln left a comment

Choose a reason for hiding this comment

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

Thanks for adding the phased CI tests 👍

@nklincoln nklincoln merged commit ff99335 into hyperledger-caliper:master Nov 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR/under review The PR is currently under review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants