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

Configuration details pertaining to Ethereum contracts should not be in the network configuration #831

Open
benjamincburns opened this issue May 6, 2020 · 3 comments
Labels
component/ethereum Related to the Ethereum adapter enhancement New feature or request

Comments

@benjamincburns
Copy link
Contributor

benjamincburns commented May 6, 2020

Context

In the case of Ethereum networks, contracts are considered data, and aren't really part of the definition of the network structure. This is apparent from the fact that contracts can be deployed or destroyed at any time during the lifetime of the network. At present we have some details of the contracts defined in the network configuration. Specifically, these details are the paths to the contract description files (the JSON files containing the ABI, bytecode, etc), and the gas required to execute each of the functions in the contracts.

Since contracts can be deployed by the benchmark callback files, it seems to me that details about the contract configuration should be defined in the contract description files, not the network configuration, and the path to the contract description file should be moved into the benchmark configuration.

Expected Behavior

I should be able to maintain a repository of benchmarks separately from my repository of deployment automation for SUTs, such that I can deploy and execute both my network and my benchmark without additional automation steps to modify network configs to include updated contract details.

Actual Behavior

I can't maintain a clean separation of concerns between configuration of the structure of my network, and configuration of the requirements of my benchmark.

Possible Fix

  1. Move the gas requirements into the contract description file (the JSON file containing the ABI, bytecode, etc).
  2. Move the path to the contract description file into the benchmark configuration file.

Context

I am maintaining the benchmark automation and tooling used by PegaSys for the purpose of monitoring and improving the performance of Hyperledger Besu. At the moment we use Terraform to provision and deploy a fresh network, and to provision and deploy cloud VMs on which Caliper runs. I'd like to maintain a repository of Besu configs and Terraform provisioning configurations for a bunch of different Besu setups, and another separate repository that contains my benchmark configurations.

Ideally the network configuration files used by Caliper would be defined alongside the configurations for the various networks that we'd like to test, as this is the most logical place for them to be versioned (if one changes, so must the other). As things stand today, key portions of contract metadata for a given benchmark are stored in the network configuration file, meaning that I can't decouple my benchmarks from my infrastructure configuration.

Your Environment

  • Version used: 0.3.0, 0.3.1 (prerelease)
@aklenik
Copy link
Contributor

aklenik commented May 13, 2020

Gave some thoughts to this. This is a general problem for all adapters. As the issue mentions, the core problem is fixing the network-contracts pairing, which should be independent. A complete decoupling would be good. If we use a reference in the network configuration to a contract configuration, the pairing is still fixed in one direction.

So what I propose is the following: make the contract configuration completely separate from the network configuration. We can introduce a caliper-contractconfig setting key, similar to the current caliper-networkconfig key. So the two configs are really orthogonal. Whether complete separation is possible, that depends on the DLT, but the current situation can be improved.

@aklenik aklenik added the enhancement New feature or request label May 13, 2020
@aklenik aklenik added this to the v0.4.0 milestone May 13, 2020
@benjamincburns
Copy link
Contributor Author

I like this proposal. It's a bit more work, and breaks more things, but I think it cleans things up better than what I'd proposed.

@aklenik
Copy link
Contributor

aklenik commented Jun 4, 2020

I think the implementation difficulties are similar. At some point, you need to load another file's content (describing the contracts) and process it. It shouldn't make much difference whether you get the file path from a network config attribute, or from a runtime setting.
(That's the theory, and we'll see how the practice turns out 😄 )

@nklincoln nklincoln modified the milestones: v0.4.0, Future release Aug 26, 2020
@nklincoln nklincoln added the component/ethereum Related to the Ethereum adapter label Oct 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/ethereum Related to the Ethereum adapter enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants