Go quorum interop #1782

pinges · 2021-01-11T07:11:52Z

PR description

Make Besu interop with Quorum privacy

…in privacy) Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net>

Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net>

…mTransaction

Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net>

…mTransaction

Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net>

…mTransaction

Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net>

…mTransaction

Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net>

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

…mTransaction

…block'. Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net>

…mTransaction

Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net>

…erop

…mTransaction

Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net>

…erop

Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net>

…erop

Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net>

…erop

Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net>

…erop

Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net>

…erop

config/besu1/key

ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/RpcApis.java

.../src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/response/JsonRpcError.java

macfarla · 2021-01-21T00:14:55Z

...perledger/besu/ethereum/api/jsonrpc/websocket/subscription/logs/LogsSubscriptionService.java

@@ -32,6 +32,8 @@
  private final SubscriptionManager subscriptionManager;
  private final Optional<PrivacyQueries> privacyQueries;

+  // TODO-goquorum GoQuorum private txs events


is this TODO staying?

do we need a GI for it

We need to find out how this is done in GoQuorum and then we can decide what to do with the TODO. We might need a GI for that then.

I think I know what we have to do to support events for the private transactions. I will remove the TODO and add extra information to the ticket that we have.

macfarla · 2021-01-21T00:21:45Z

...esu/ethereum/api/jsonrpc/internal/privacy/methods/eea/GoQuorumSendRawPrivateTransaction.java

+
+  @Override
+  public String getName() {
+    return RpcMethod.ETH_SEND_RAW_PRIVATE_TRANSACTION.getMethodName();


does eea_sendRaw use same method name as eth_sendRaw?

They came up with this new API (eth_sendRawPrivateTransaction)

should this one be under the eea package then or would it make sense to move it out?

It is not an eea API ...

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/ProtocolContext.java

...um/core/src/main/java/org/hyperledger/besu/ethereum/core/encoding/TransactionRLPDecoder.java

macfarla · 2021-01-21T00:39:57Z

...rc/test/java/org/hyperledger/besu/ethereum/eth/messages/LimitedTransactionsMessagesTest.java

+                    .isLessThan(
+                        LimitedTransactionsMessages.LIMIT
+                            + MAX_ADDITIONAL_BYTES)); // TODO: Sally/Mark/Lucas please check that 5
+    // is really the max!


wait what is this limit?

In LimitedTransactionsMessages they use LimitedTransactionsMessages.LIMIT to check how many transactions they can add, but once the limit is reached there are some bytes added with the message.endList(). That means that the size can be bigger than LimitedTransactionsMessages.LIMIT, and I think you can have a maximum of 5 additional bytes ...

Do we need a test to ensure we handle this scenario nicely?

I've had a look at the code again and I think that the 5 additional bytes are right. I've removed the TODO! I don't think we need another test ...

besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java

Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net>

…erop

mark-terry

LGTM. Should some of those ToDos be resolved?

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/ProtocolSpecBuilder.java

mark-terry · 2021-01-27T00:16:09Z

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/goquorum/GoQuorumKeyValueStorage.java

+
+public class GoQuorumKeyValueStorage implements GoQuorumPrivateStorage {
+
+  // TODO-goquorum We need to think about whether we have to have the atomic commit of public and


Does this require a decision to be made?

Yes, we should probably discuss that once the PR is merged! I'll leave the TODO in to remind us!

mark-terry · 2021-01-27T00:21:27Z

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/goquorum/GoQuorumPrivateStorage.java

+
+  interface Updater {
+
+    // TODO-goquorum Do we need this? I think in quorum they do not have both the public and the


Need to discuss?

I think that we agreed that we do not need that mapping anymore. Will leave the TODO here and fix that when this PR is merged!

mark-terry · 2021-01-27T00:21:37Z

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/goquorum/GoQuorumPrivateStorage.java

+    Updater putPrivateStateRootHashMapping(
+        final Hash publicStateRootHash, final Hash privateStateRootHash);
+
+    // TODO-goquorum Do we need this? I think we are storing the private Tx receipts instead of the


Need to discuss?

I think that we agreed that we do not need that mapping anymore. Will leave the TODO here and fix that when this PR is merged!

lucassaldanha · 2021-01-27T00:46:50Z

ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/RpcApis.java

@@ -64,6 +65,8 @@
      return Optional.of(TRACE);
    } else if (name.equals(PLUGINS.getCliValue())) {
      return Optional.of(PLUGINS);
+    } else if (name.equals(GOQUORUM.getCliValue())) {


Is this leaving up to the user to enable the GOQUORUM methods? Do we really need to do this? We should always use the GoQuorum "versions" of the methods if GoQuorum mode is enabled.

That would be an easy change and we should discuss that. Would prefer to do that after the PR is merged!

lucassaldanha · 2021-01-27T00:51:38Z

.../src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/response/JsonRpcError.java

+      -50100, "No privateFor specified in rawTxArgs for GoQuorum raw private transaction."),
+  GOQUORUM_ONLY_STANDARD_MODE_SUPPORTED(
+      -50100,
+      "Mode other than 'standard' mode defined in rawTxArgs for GoQuorum raw private transaction."),


This error msg seems a bit confusing. It could say: "Invalid private transaction mode" or something like that.

lucassaldanha · 2021-01-27T01:29:10Z

...rc/test/java/org/hyperledger/besu/ethereum/eth/messages/LimitedTransactionsMessagesTest.java

+                    .isLessThan(
+                        LimitedTransactionsMessages.LIMIT
+                            + MAX_ADDITIONAL_BYTES)); // TODO: Sally/Mark/Lucas please check that 5
+    // is really the max!


Do we need a test to ensure we handle this scenario nicely?

Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net>

…erop

Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net>

…erop

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/privacy/GoQuorumSendRawTxArgs.java

RatanRSur · 2021-01-28T15:13:16Z

...rc/test/java/org/hyperledger/besu/ethereum/eth/messages/LimitedTransactionsMessagesTest.java

-            messageSize -> assertThat(messageSize).isLessThan(LimitedTransactionsMessages.LIMIT));
+            messageSize ->
+                assertThat(messageSize)
+                    .isLessThan(LimitedTransactionsMessages.LIMIT + MAX_ADDITIONAL_BYTES));


Why can't we stay under the LIMIT anymore?

We can get over the limit because we are checking the length of the message against LimitedTransactionsMessages.LIMIT before we are adding the length of that RLP encoded list.
Looking at commit messages it looks like this is a self imposed limit. It changes from limiting the number of transactions to limiting the size. I guess that means that it should not matter if there are 5 bytes more. I will slack Abdel to make sure I'm right.

Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net>

macfarla

LGTM

* Enable Besu to import blocks containing quorum style private transactions * Add RPC to accept quorum style raw private transactions Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net> Signed-off-by: Lucas Saldanha <lucascrsaldanha@gmail.com> Co-authored-by: Lucas Saldanha <lucascrsaldanha@gmail.com>

pinges and others added 22 commits November 25, 2020 11:13

separate current functionality into two classes (on-chain and off-cha…

8e524d1

…in privacy) Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net>

separate current functionality into two classes (on-chain and off-cha…

31ab133

…in privacy) Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net>

separate current functionality into two classes (on-chain and off-cha…

eb693b1

…in privacy) Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net>

changes based on comments

e3631a9

Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net>

changes based on comments

a760ba0

Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net>

Merge branch 'master' of github.com:hyperledger/besu into createQuoru…

1970507

…mTransaction

changes based on comments

d254e22

Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net>

Merge branch 'master' of github.com:hyperledger/besu into createQuoru…

5a37b4b

…mTransaction

Merge branch 'master' of github.com:hyperledger/besu into createQuoru…

700eff0

…mTransaction

add quorum raw private transactions

55e59d4

Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net>

Merge branch 'master' of github.com:hyperledger/besu into createQuoru…

2985980

…mTransaction

some changes

424a33d

Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net>

Merge branch 'master' of github.com:hyperledger/besu into createQuoru…

b12bc00

…mTransaction

merge privacy configuration

dbbd27e

Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net>

Initial goquorum private tx processing

2c43940

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

Added GoQuorumKeyValueStorage

119a9d2

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

Removed unecessary classes

4adcf0b

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

Clenup

8774a89

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

Merge branch 'master' of github.com:hyperledger/besu into createQuoru…

28687be

…mTransaction

private Tx mined by quorum, but import into besu fails with 'invalid …

2b0fefc

…block'. Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net>

Merge branch 'master' of github.com:hyperledger/besu into createQuoru…

a785608

…mTransaction

tests passing after rebase with Lucas branch

5e4635f

Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net>

pinges requested a review from lucassaldanha January 11, 2021 07:12

pinges added privacy private transactions TeamRevenant GH issues worked on by Revenant Team labels Jan 11, 2021

pinges added 5 commits January 12, 2021 08:59

spotless

d2e2506

Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net>

merged in changes from master. All tests passing

b0a1aed

Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net>

Merge branch 'master' of github.com:hyperledger/besu into goQuorumInt…

410cd83

…erop

Merge branch 'master' of github.com:hyperledger/besu into goQuorumInt…

4af37ab

…erop

Merge branch 'master' of github.com:hyperledger/besu into createQuoru…

47e9c11

…mTransaction

pinges added 12 commits January 19, 2021 11:23

fix AT NullPointerExceptions

365618b

Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net>

Merge branch 'master' of github.com:hyperledger/besu into goQuorumInt…

f45b539

…erop

remove another static field

424dff5

Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net>

some cleanup

b0db717

Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net>

Merge branch 'master' of github.com:hyperledger/besu into goQuorumInt…

ffa56b3

…erop

more cleanup

bafc3d6

Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net>

even more cleanup

1cad6ce

Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net>

Merge branch 'master' of github.com:hyperledger/besu into goQuorumInt…

0042fdf

…erop

and even more cleanup

da6c97e

Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net>

Merge branch 'master' of github.com:hyperledger/besu into goQuorumInt…

17eec25

…erop

fix a unit test

1fb251d

Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net>

Merge branch 'master' of github.com:hyperledger/besu into goQuorumInt…

c806616

…erop

macfarla reviewed Jan 21, 2021

View reviewed changes

pinges added 2 commits January 22, 2021 17:46

fixes after review from Sally

314234e

Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net>

Merge branch 'master' of github.com:hyperledger/besu into goQuorumInt…

7e7ebd3

…erop

mark-terry reviewed Jan 27, 2021

View reviewed changes

lucassaldanha reviewed Jan 27, 2021

View reviewed changes

pinges added 4 commits January 27, 2021 12:30

merge master and somes fixes after review from Mark

73e96f2

Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net>

changes after comments from Lucas

97dfe34

Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net>

Merge branch 'master' of github.com:hyperledger/besu into goQuorumInt…

64ab12e

…erop

fix spotless

a9d583e

Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net>

pinges mentioned this pull request Jan 27, 2021

Interop with GoQuorum privacy #1742

Closed

RatanRSur self-requested a review January 27, 2021 15:01

Merge branch 'master' of github.com:hyperledger/besu into goQuorumInt…

d08eb35

…erop

macfarla reviewed Jan 28, 2021

View reviewed changes

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/privacy/GoQuorumSendRawTxArgs.java Outdated Show resolved Hide resolved

RatanRSur reviewed Jan 28, 2021

View reviewed changes

remove some comments

8710904

Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net>

macfarla approved these changes Jan 29, 2021

View reviewed changes

pinges merged commit 78fe588 into hyperledger:master Jan 29, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Go quorum interop #1782

Go quorum interop #1782

pinges commented Jan 11, 2021

macfarla Jan 21, 2021

macfarla Jan 21, 2021

pinges Jan 21, 2021

pinges Jan 25, 2021

macfarla Jan 21, 2021

pinges Jan 21, 2021

macfarla Jan 22, 2021

pinges Jan 25, 2021

macfarla Jan 21, 2021

pinges Jan 22, 2021

lucassaldanha Jan 27, 2021

pinges Jan 27, 2021

mark-terry left a comment •

edited

Loading

mark-terry Jan 27, 2021

pinges Jan 27, 2021

mark-terry Jan 27, 2021

pinges Jan 27, 2021

mark-terry Jan 27, 2021

pinges Jan 27, 2021

lucassaldanha Jan 27, 2021

pinges Jan 27, 2021

lucassaldanha Jan 27, 2021

pinges Jan 27, 2021

lucassaldanha Jan 27, 2021

RatanRSur Jan 28, 2021

pinges Jan 28, 2021

macfarla left a comment


		public class GoQuorumKeyValueStorage implements GoQuorumPrivateStorage {

		// TODO-goquorum We need to think about whether we have to have the atomic commit of public and


		interface Updater {

		// TODO-goquorum Do we need this? I think in quorum they do not have both the public and the

Go quorum interop #1782

Go quorum interop #1782

Conversation

pinges commented Jan 11, 2021

PR description

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mark-terry left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

macfarla left a comment

Choose a reason for hiding this comment

mark-terry left a comment •

edited

Loading