Implementation of Issue #1497 - Allow the Ethereum connector to use an already deployed contract

[rmarede]

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

Issue #1497 - Allow the Ethereum connector to use an already deployed contract

Existing issues

• GitHub Issues

Design of the fix

To use a contract that is already deployed, an optional 'address' key may be specified on the Contract Definition file. The new connector reads the 'address' key from the Contract Definition File, and if it exists, it does not deploy a new contract.

Validation of the fix

Working on my local Besu network

[davidkel]

I believe the way to solve this in caliper would be to use the --caliper-flow-skip-install option when launching caliper. This would skip the installing of the smart contract and thus allow it to work with an already deployed contract. I would also be interested to know if you can also skip the init phase as well --caliper-flow-skip-init as I believe this phase is only required to install a smart contract. Personally I think caliper should only work with pre-deployed contracts and not perform any attempt to install whatsoever and the proposal is here #1586. So at this time I don't think we need this code change (and also it would need both unit tests and integration tests proving it works if it can be considered for merging in the future)

I can't say whether it would be considered or not at this time, but given there are no tests at present it simply cannot even be considered at the moment. If you want it to be considered as a possible alternative to the skip-install option then please add appropriate integration tests at least and ideally unit tests as well.

[rmarede]

Assuming you go with your proposal in #1586, the deployed contract address must still be provided to the ethereum connector.
So I wonder where will this information be specified. I think the Contract Definition file is a good fit. So I don't see the 2 proposals as mutually exclusive. But I could be wrong, I didn't find much about that --caliper-flow-skip-install option.

Regarding the validation of this proposal, I understand that it's hard to consider without unit and integration tests. But at this time there aren't even such tests for the ethereum connector, and I currently don't have the time to create them from scratch. Also, it has been 2 years since someone modified its implementation and it is still very incomplete and has a lot of TODO's in it. From what I see, the ethereum connector does not even work properly when using more than 1 worker...

[davidkel]

I believe the ethereum connector will work with already deployed contracts. I believe that is pretty much what people have been told in the past due to the lack of capability in caliper to deploy contracts, so have you tested using the skip option to see if it works for your environment ?
Admittedly there are no unit tests for this, but there are integration tests and it's not unreasonable to ask committers to provide tests, so by extending the integration tests and creating the initial unit tests as none exist currently. Unless we make sure tests are written then tests never get written. It must be the responsibility of the code writer to provide appropriate tests otherwise we just cannot accept the commit. I agree that the ethereum connector is very much out of date. It's not my area but unfortunately we don't have a maintainer now who is skilled in this area. We really need help from people on this or we will consider just dropping ethereum altogether and leave caliper as a pure Hyperledger Fabric benchmark tool.

[davidkel]

I've added issue #1589 which should show how to work with an already deployed contract, so no code changes required. But just looking at the documentation it's clear to me the ethereum connector docs need an overhaul to really improve them.

[rmarede]

Ah, I see. I didn't know you could add the address in the network config and do it that way. This really needs to be documented. Thanks for opening issue #1589, I am closing this PR.

But before that, would you happen to know anything about what I said previously on the ethereum connector not working with more than 1 worker? I know this is not your area but maybe I'm doing something wrong or this is intended for some reason. I just posted a question on the caliper discord channel with more details on the problem. I'm considering opening an issue on this.

[davidkel]

@rmarede I bet it's again something that should be in the documentation but isn't. If I look at https://github.com/hyperledger/caliper-benchmarks/blob/main/networks/besu/1node-clique/erc721networkconfig.json I see they use fromAddressSeed which looks like it would give each worker a different address ? Those tests all use a single worker, but the besu integration test in caliper itself has a phase that uses 2 workers and it also uses fromAddressSeed so hopefully that helps

[rmarede]

Using a different address for each worker should fix this issue. Thank you.