Added support for Besu private transactions #1030

lucassaldanha · 2020-10-01T23:14:44Z

Besu supports private transactions sent to a list of participant nodes. The library web3js-eea supports sending private transactions to Besu.

By expanding the network configuration DSL and adding a few changes to the Ethereum connection, we can support sending private transactions to Besu using Caliper.

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 change
How to prove that the change works
Automated tests that prove the fix keeps on working
Documentation - any JSDoc, website, or Stackoverflow answers?

Issue/User story

As a user, I want to have performance metrics of Besu when using private transactions.

Steps to Reproduce

N/A

Existing issues

N/A

Design of the change

The core of the fix was to update ethereum-connector.js to be able to send private transactions. The network config DSL was extended to support some Besu privacy configurations.

In the network configuration, the ethereum object now can have a privacy section, enumerating privacy groups in the network. Also, there is the option of specifying what contracts should be private (and to what privacy group - aka participants - they belong to).

In the Ethereum connection, there are two new methods exclusively used for private transactions: deployPrivateContract and _sendSinglePrivateRequest.

Validation of the change

I added a new step in the Besu integration tests to check the basic flow of public vs private contracts. I have done some manual testing as well to ensure the changes work.

Automated Tests

See Validation of the change

What documentation has been provided for this pull request

I'm not sure if we have specific Besu documentation for Caliper. If we do, we could probably add some info about using Besu with private transactions. Otherwise, the integration tests should be a good start for anyone wanting to use it.

lgtm-com · 2020-10-01T23:22:17Z

This pull request introduces 3 alerts when merging ba7cb7b into 5938426 - view on LGTM.com

new alerts:

2 for Expression has no effect
1 for Unused variable, import, function or class

benjamincburns · 2020-10-01T23:29:37Z

packages/caliper-ethereum/lib/ethereum-connector.js

+            let contract = this.ethereumConfig.contracts[key];
+            let contractData = require(CaliperUtils.resolvePath(contract.path)); // TODO remove path property
+            let contractGas = contract.gas;
+            let estimateGas = contract.estimateGas;


These could be const - helps the JIT optimizer

benjamincburns

LGTM - only one comment that's really inconsequential. Thanks a ton for including the new tests!

lgtm-com · 2020-10-02T01:32:01Z

This pull request introduces 4 alerts when merging 8a74c7d into 5938426 - view on LGTM.com

new alerts:

2 for Expression has no effect
1 for Assignment to constant
1 for Syntax error

lgtm-com · 2020-10-02T01:41:24Z

This pull request introduces 2 alerts when merging c3a5701 into 5938426 - view on LGTM.com

new alerts:

2 for Expression has no effect

lgtm-com · 2020-10-02T01:52:05Z

This pull request introduces 1 alert when merging fafd1a5 into 5938426 - view on LGTM.com

new alerts:

1 for Expression has no effect

lgtm-com · 2020-10-02T02:17:44Z

This pull request introduces 1 alert when merging b565db9 into 5938426 - view on LGTM.com

new alerts:

1 for Expression has no effect

Signed-off-by: Lucas Saldanha <lucascrsaldanha@gmail.com>

lgtm-com · 2020-10-02T02:33:31Z

This pull request introduces 1 alert when merging 99e337c into 5938426 - view on LGTM.com

new alerts:

1 for Expression has no effect

nklincoln · 2020-10-02T09:53:03Z

@lucassaldanha - there's a few LGTM bot warnings present at the moment, which you can view above, and one conflict due to a recent release that will need a quick rebase 👍

Signed-off-by: Lucas Saldanha <lucascrsaldanha@gmail.com>

nklincoln · 2020-10-05T12:13:01Z

@lucassaldanha - thanks for the update, there is one pending LGTM issue relating to the store.js test file in the constructor (this.contract has no effect) are you able to update that one too please?

lucassaldanha · 2020-10-05T20:19:31Z

thanks for the update, there is one pending LGTM issue relating to the store.js test file in the constructor (this.contract has no effect) are you able to update that one too please?

@nklincoln I thought I had it fixed might've missed it. Where are you seeing the warning?

nklincoln · 2020-10-06T08:24:51Z

@lucassaldanha - thanks, I must have been looking at an out of date notification.

I've been looking at the output of the integration test run, and there appears to be a negative send rate being reported. Would you be able to bump the transaction number to a higher number (100 perhaps) so that we can determine if it is something in the private transaction addition, or a bug in caliper for low transaction numbers?

Signed-off-by: Lucas Saldanha <lucascrsaldanha@gmail.com>

lucassaldanha · 2020-10-06T08:47:23Z

@nklincoln Done! :)

nklincoln

Hi @lucassaldanha , had a deeper look as was seeing a negative send rate ... looks to be a bug in caliper main ... but did notice the flag that might need a change

nklincoln · 2020-10-08T13:00:44Z

packages/caliper-tests-integration/besu_tests/store.js

+            this.privacyOpts = sutContext.privacy[this.roundArguments.private];
+            this.privacyOpts['id'] = this.roundArguments.private;
+        } else {
+            this.private = false;


this.isPrivate = false ?

Nice catch! Because of the way the value was being consumed, it was undefined for public txs and in the end, the logic was working out the same. Lucky! :)

Signed-off-by: Lucas Saldanha <lucascrsaldanha@gmail.com>

nklincoln · 2020-10-09T10:31:09Z

@lucassaldanha - Thanks for the contribution 👍

It would be great if you could extend the docs to include details on how to take advantage of the new features added.

If you checkout the gh-pages branch, the file of interest is doc/vNext/EthereumConfiguration.md - any questions, please reach out to us on RC 😄

* Added support for Besu private transactions Signed-off-by: Lucas Saldanha <lucascrsaldanha@gmail.com> * Fix LGTM warning Signed-off-by: Lucas Saldanha <lucascrsaldanha@gmail.com> * Updated tx count Signed-off-by: Lucas Saldanha <lucascrsaldanha@gmail.com> * Fix variable name Signed-off-by: Lucas Saldanha <lucascrsaldanha@gmail.com>

lucassaldanha force-pushed the besu-privacy branch from 6e70b11 to ba7cb7b Compare October 1, 2020 23:16

benjamincburns reviewed Oct 1, 2020

View reviewed changes

benjamincburns approved these changes Oct 1, 2020

View reviewed changes

lucassaldanha force-pushed the besu-privacy branch 2 times, most recently from 21a1463 to 8a74c7d Compare October 2, 2020 01:28

lucassaldanha force-pushed the besu-privacy branch from 8a74c7d to c3a5701 Compare October 2, 2020 01:37

lucassaldanha force-pushed the besu-privacy branch from c3a5701 to fafd1a5 Compare October 2, 2020 01:47

lucassaldanha force-pushed the besu-privacy branch from fafd1a5 to b565db9 Compare October 2, 2020 02:13

Added support for Besu private transactions

99e337c

Signed-off-by: Lucas Saldanha <lucascrsaldanha@gmail.com>

lucassaldanha force-pushed the besu-privacy branch from b565db9 to 99e337c Compare October 2, 2020 02:29

lucassaldanha added 2 commits October 5, 2020 14:26

Merge branch 'master' into besu-privacy

01bf5d6

Signed-off-by: Lucas Saldanha <lucascrsaldanha@gmail.com>

Fix LGTM warning

1776f39

Signed-off-by: Lucas Saldanha <lucascrsaldanha@gmail.com>

lucassaldanha force-pushed the besu-privacy branch from 13a514a to 1776f39 Compare October 5, 2020 01:28

Updated tx count

f88361f

Signed-off-by: Lucas Saldanha <lucascrsaldanha@gmail.com>

nklincoln suggested changes Oct 8, 2020

View reviewed changes

Fix variable name

8f524d1

Signed-off-by: Lucas Saldanha <lucascrsaldanha@gmail.com>

nklincoln approved these changes Oct 9, 2020

View reviewed changes

nklincoln merged commit 161bce5 into hyperledger:master Oct 9, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added support for Besu private transactions #1030

Added support for Besu private transactions #1030

lucassaldanha commented Oct 1, 2020

lgtm-com bot commented Oct 1, 2020

benjamincburns Oct 1, 2020

benjamincburns left a comment

lgtm-com bot commented Oct 2, 2020

lgtm-com bot commented Oct 2, 2020

lgtm-com bot commented Oct 2, 2020

lgtm-com bot commented Oct 2, 2020

lgtm-com bot commented Oct 2, 2020

nklincoln commented Oct 2, 2020

nklincoln commented Oct 5, 2020

lucassaldanha commented Oct 5, 2020

nklincoln commented Oct 6, 2020

lucassaldanha commented Oct 6, 2020

nklincoln left a comment •

edited

Loading

nklincoln Oct 8, 2020

lucassaldanha Oct 8, 2020

lucassaldanha Oct 8, 2020

nklincoln commented Oct 9, 2020

Added support for Besu private transactions #1030

Added support for Besu private transactions #1030

Conversation

lucassaldanha commented Oct 1, 2020

Checklist

Issue/User story

Steps to Reproduce

Existing issues

Design of the change

Validation of the change

Automated Tests

What documentation has been provided for this pull request

lgtm-com bot commented Oct 1, 2020

benjamincburns Oct 1, 2020

Choose a reason for hiding this comment

benjamincburns left a comment

Choose a reason for hiding this comment

lgtm-com bot commented Oct 2, 2020

lgtm-com bot commented Oct 2, 2020

lgtm-com bot commented Oct 2, 2020

lgtm-com bot commented Oct 2, 2020

lgtm-com bot commented Oct 2, 2020

nklincoln commented Oct 2, 2020

nklincoln commented Oct 5, 2020

lucassaldanha commented Oct 5, 2020

nklincoln commented Oct 6, 2020

lucassaldanha commented Oct 6, 2020

nklincoln left a comment • edited Loading

Choose a reason for hiding this comment

nklincoln Oct 8, 2020

Choose a reason for hiding this comment

lucassaldanha Oct 8, 2020

Choose a reason for hiding this comment

lucassaldanha Oct 8, 2020

Choose a reason for hiding this comment

nklincoln commented Oct 9, 2020

nklincoln left a comment •

edited

Loading