Tracing private transactions feature #6161

NickSneo · 2023-11-12T04:50:19Z

PR description

PR for adding priv_traceTransaction API

Fixed Issue(s)

#5280

github-actions · 2023-11-12T04:50:35Z

I thought about documentation and added the doc-change-required label to this PR if updates are required.
I thought about the changelog and included a changelog update if required.
If my PR includes database changes (e.g. KeyValueSegmentIdentifier) I have thought about compatibility and performed forwards and backwards compatibility tests

macfarla · 2023-11-13T02:20:30Z

Hi @NickSneo - would like to see some unit and acceptance tests for this new method!

NickSneo · 2023-11-13T02:38:23Z

Hi @NickSneo - would like to see some unit and acceptance tests for this new method!

Hey @macfarla , Sure. I am just working on it. Will push the new commits by the end of this week.

pinges

I've noticed that there is a lot of code that is very similar to existing code (e.g. see PrivateTraceGenerator vs FlatTraceGenerator).
I think it would be good to try to have less code duplication.

.../java/org/hyperledger/besu/tests/acceptance/dsl/privacy/transaction/PrivacyTransactions.java

...t/java/org/hyperledger/besu/tests/acceptance/privacy/PrivTraceTransactionAcceptanceTest.java

...dger/besu/ethereum/api/jsonrpc/internal/privacy/methods/priv/PrivateAbstractTraceByHash.java

NickSneo · 2024-04-30T05:27:15Z

I've noticed that there is a lot of code that is very similar to existing code (e.g. see PrivateTraceGenerator vs FlatTraceGenerator). I think it would be good to try to have less code duplication.

Hey @pinges ,

Thank you for reviewing the PR. I have resolved all the comments. For code duplication I have tried to have as less as possible, but since we want to make private and public code separate from each other, I have followed the similar approach as it is right now -> some code duplication is expected. Let me know if it is fine or I still need to change.

Also regarding the failing pipeline, It is not related to any of my change, should I try to fix that too in this PR?

NickSneo · 2024-05-07T17:08:06Z

Hey @pinges @macfarla ,
Please review this PR

I've noticed that there is a lot of code that is very similar to existing code (e.g. see PrivateTraceGenerator vs FlatTraceGenerator). I think it would be good to try to have less code duplication.

Hey @pinges ,

Thank you for reviewing the PR. I have resolved all the comments. For code duplication I have tried to have as less as possible, but since we want to make private and public code separate from each other, I have followed the similar approach as it is right now -> some code duplication is expected. Let me know if it is fine or I still need to change.

Also regarding the failing pipeline, It is not related to any of my change, should I try to fix that too in this PR?

...java/org/hyperledger/besu/tests/acceptance/dsl/transaction/privacy/PrivTraceTransaction.java

plugin-api/src/main/java/org/hyperledger/besu/plugin/data/PrivateTransaction.java

macfarla · 2024-05-16T05:27:34Z

Hey @pinges @macfarla , Please review this PR

I've noticed that there is a lot of code that is very similar to existing code (e.g. see PrivateTraceGenerator vs FlatTraceGenerator). I think it would be good to try to have less code duplication.

Hey @pinges ,
Thank you for reviewing the PR. I have resolved all the comments. For code duplication I have tried to have as less as possible, but since we want to make private and public code separate from each other, I have followed the similar approach as it is right now -> some code duplication is expected. Let me know if it is fine or I still need to change.
Also regarding the failing pipeline, It is not related to any of my change, should I try to fix that too in this PR?

would it make sense to have an abstract class that FlatTrace can extend?

For the failing test - i have created an issue for it #7108 - that is not caused by this PR - but a lot of those permissioning tests are flaky lately - so if you feel like fixing them separately that would be super!

NickSneo · 2024-05-17T15:42:27Z

would it make sense to have an abstract class that FlatTrace can extend?

For the failing test - i have created an issue for it #7108 - that is not caused by this PR - but a lot of those permissioning tests are flaky lately - so if you feel like fixing them separately that would be super!

@macfarla Sure, will create an abstract class that FlatTrace can extend. Also will look into the issue #7108

NickSneo · 2024-06-10T07:38:10Z

would it make sense to have an abstract class that FlatTrace can extend?

For the failing test - i have created an issue for it #7108 - that is not caused by this PR - but a lot of those permissioning tests are flaky lately - so if you feel like fixing them separately that would be super!

Hey @macfarla ,
I tried two approaches for FlatTraceGenerator and PrivateTraceGenerator -

Abstract Class - But the signatures are different, and I tried to have a common signature but then goes into a rabbit hole to change a lot of classes but still failed to resolve few issues like handling ExecutedPrivateTransaction and Transaction.
PrivateTraceGenerator inheriting FlatTraceGenerator - In this too failed to override common methods since singatures are different.

Right now not trying to modify logic for FlatTraceGenerator and PrivateTraceGenerator.

Also, Can we get this merged soon, so I can pick other issues too. Thanks

Edit: Any change in PrivateFlatTrace file will require whole refactoring and a lot of changes in current PR. Current solution works as expected, I think we should merge it as it is

NickSneo · 2024-07-03T08:40:53Z

Hey @macfarla @non-fungible-nelson ,

Can we get this PR merged soon? It is open from a long time, also I have spent a lot of time on it so it would be great to see this merged. Thanks!

pinges

Please have a look at the comments.

...ledger/besu/ethereum/api/jsonrpc/internal/processor/privateProcessor/PrivateBlockReplay.java

...dger/besu/ethereum/api/jsonrpc/internal/privacy/methods/priv/AbstractPrivateTraceByHash.java

...ledger/besu/ethereum/api/jsonrpc/internal/processor/privateProcessor/PrivateBlockTracer.java

...ain/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/results/tracing/flat/Action.java

plugin-api/build.gradle

...dger/besu/ethereum/api/jsonrpc/internal/privacy/methods/priv/AbstractPrivateTraceByHash.java

...hyperledger/besu/ethereum/api/jsonrpc/internal/processor/privateProcessor/PrivateTracer.java

...besu/ethereum/api/jsonrpc/internal/results/privacy/privateTracing/PrivateTraceGenerator.java

pinges

@NickSneo I think we are very close to get this merged. If my comments about the gas do not make sense just let me know :-)

NickSneo · 2024-07-22T07:45:26Z

@NickSneo I think we are very close to get this merged. If my comments about the gas do not make sense just let me know :-)

Hey @pinges , Thanks for reviewing. Comments make sense, actually I got stuck in other Besu PR - EIP196 constantine bindings. I will resolve the comments and push the changes

NickSneo · 2024-07-26T08:23:40Z

Hey @pinges Sorry for the delay on this, I just pushed new commits to resolved comments. Please review.
Thanks!

macfarla

needs a changelog entry

Signed-off-by: Nischal Sharma <nischal@web3labs.com>

…acceptance/dsl/privacy/transaction/PrivacyTransactions.java Co-authored-by: Stefan Pingel <16143240+pinges@users.noreply.github.com> Signed-off-by: Nischal Sharma <nischal@web3labs.com>

…s/acceptance/privacy/PrivTraceTransactionAcceptanceTest.java Co-authored-by: Stefan Pingel <16143240+pinges@users.noreply.github.com> Signed-off-by: Nischal Sharma <nischal@web3labs.com>

Signed-off-by: Nischal Sharma <nischal@web3labs.com>

…ateTransaction.java Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com> Signed-off-by: Nischal Sharma <nischal@web3labs.com>

…acceptance/dsl/transaction/privacy/PrivTraceTransaction.java Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com> Signed-off-by: Nischal Sharma <nischal@web3labs.com>

Signed-off-by: Nischal Sharma <nischal@web3labs.com>

NickSneo · 2024-08-01T08:57:19Z

@pinges @macfarla Hey, I have resolved all the comments kindly review.

Thanks again!

NickSneo · 2024-08-12T16:09:07Z

@macfarla @pinges Hey, I have resolved all the comments. Waiting for you review and if we can get this merged?

macfarla

for me it looks good, @pinges can you confirm?

pinges

LGTM

Signed-off-by: Stefan Pingel <16143240+pinges@users.noreply.github.com>

* add private tx tracing feature Signed-off-by: Nischal Sharma <nischal@web3labs.com> Signed-off-by: Stefan Pingel <16143240+pinges@users.noreply.github.com> Co-authored-by: Stefan Pingel <16143240+pinges@users.noreply.github.com> Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com> Signed-off-by: gconnect <agatevureglory@gmail.com>

NickSneo mentioned this pull request Nov 12, 2023

Feature addition for tracing private transactions #5534

Closed

NickSneo marked this pull request as draft November 21, 2023 13:34

NickSneo force-pushed the nicks/private-tracing branch from d575a2c to 2745344 Compare November 23, 2023 08:19

NickSneo marked this pull request as ready for review November 23, 2023 08:46

non-fungible-nelson requested a review from pinges February 5, 2024 22:25

pinges requested changes Feb 27, 2024

View reviewed changes

NickSneo force-pushed the nicks/private-tracing branch from 2745344 to c5df48b Compare April 19, 2024 06:58

NickSneo force-pushed the nicks/private-tracing branch 2 times, most recently from 45c5137 to cc38374 Compare April 29, 2024 14:38

NickSneo requested a review from pinges April 30, 2024 05:27

macfarla reviewed May 16, 2024

View reviewed changes

...java/org/hyperledger/besu/tests/acceptance/dsl/transaction/privacy/PrivTraceTransaction.java Outdated Show resolved Hide resolved

plugin-api/src/main/java/org/hyperledger/besu/plugin/data/PrivateTransaction.java Outdated Show resolved Hide resolved

NickSneo force-pushed the nicks/private-tracing branch from d129369 to db7f44b Compare July 3, 2024 08:16

NickSneo requested a review from macfarla July 9, 2024 14:02

pinges reviewed Jul 9, 2024

View reviewed changes

pinges reviewed Jul 22, 2024

View reviewed changes

NickSneo force-pushed the nicks/private-tracing branch from 4b12870 to 545458a Compare July 26, 2024 08:15

NickSneo requested a review from pinges July 26, 2024 08:22

NickSneo force-pushed the nicks/private-tracing branch from 7f09252 to a73da2d Compare July 29, 2024 14:34

macfarla reviewed Jul 30, 2024

View reviewed changes

NickSneo and others added 16 commits August 1, 2024 14:15

fix for acceptance test

0c36e39

Signed-off-by: Nischal Sharma <nischal@web3labs.com>

resolve conflicts

8e29a92

Signed-off-by: Nischal Sharma <nischal@web3labs.com>

Update acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/…

7e916f6

…acceptance/dsl/privacy/transaction/PrivacyTransactions.java Co-authored-by: Stefan Pingel <16143240+pinges@users.noreply.github.com> Signed-off-by: Nischal Sharma <nischal@web3labs.com>

Update acceptance-tests/tests/src/test/java/org/hyperledger/besu/test…

cf0a3c0

…s/acceptance/privacy/PrivTraceTransactionAcceptanceTest.java Co-authored-by: Stefan Pingel <16143240+pinges@users.noreply.github.com> Signed-off-by: Nischal Sharma <nischal@web3labs.com>

fix PrivJsonRpcMethodsTest

e8feb00

Signed-off-by: Nischal Sharma <nischal@web3labs.com>

Added more tests and fail scenarios

8645558

Signed-off-by: Nischal Sharma <nischal@web3labs.com>

test fix

73d5648

Signed-off-by: Nischal Sharma <nischal@web3labs.com>

test fixes

42c9d3f

Signed-off-by: Nischal Sharma <nischal@web3labs.com>

revert changes

72be001

Signed-off-by: Nischal Sharma <nischal@web3labs.com>

spotless apply

2905612

Signed-off-by: Nischal Sharma <nischal@web3labs.com>

Update plugin-api/src/main/java/org/hyperledger/besu/plugin/data/Priv…

585a30d

…ateTransaction.java Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com> Signed-off-by: Nischal Sharma <nischal@web3labs.com>

Update acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/…

43d44c4

…acceptance/dsl/transaction/privacy/PrivTraceTransaction.java Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com> Signed-off-by: Nischal Sharma <nischal@web3labs.com>

spotless apply

8e6079c

Signed-off-by: Nischal Sharma <nischal@web3labs.com>

resolved comments

eb1cc13

Signed-off-by: Nischal Sharma <nischal@web3labs.com>

changelog added and invalid_param catch

722859a

Signed-off-by: Nischal Sharma <nischal@web3labs.com>

resolved conflict

43d8d80

Signed-off-by: Nischal Sharma <nischal@web3labs.com>

NickSneo force-pushed the nicks/private-tracing branch from 1a7e806 to 43d8d80 Compare August 1, 2024 08:55

NickSneo requested review from pinges and macfarla August 1, 2024 08:59

macfarla approved these changes Aug 13, 2024

View reviewed changes

pinges approved these changes Aug 13, 2024

View reviewed changes

Merge branch 'main' into nicks/private-tracing

7ad2bfe

Signed-off-by: Stefan Pingel <16143240+pinges@users.noreply.github.com>

pinges enabled auto-merge (squash) August 13, 2024 01:44

Merge branch 'main' into nicks/private-tracing

eb773ce

pinges merged commit 50f8add into hyperledger:main Aug 13, 2024
40 checks passed

joaniefromtheblock mentioned this pull request Aug 15, 2024

Tracing private transactions feature hyperledger/besu-docs#1669

Closed

joaniefromtheblock removed the doc-change-required Indicates an issue or PR that requires doc to be updated label Aug 15, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tracing private transactions feature #6161

Tracing private transactions feature #6161

NickSneo commented Nov 12, 2023

github-actions bot commented Nov 12, 2023 •

edited by NickSneo

Loading

macfarla commented Nov 13, 2023

NickSneo commented Nov 13, 2023

pinges left a comment

NickSneo commented Apr 30, 2024

NickSneo commented May 7, 2024

macfarla commented May 16, 2024

NickSneo commented May 17, 2024

NickSneo commented Jun 10, 2024 •

edited

Loading

NickSneo commented Jul 3, 2024

pinges left a comment

pinges left a comment

NickSneo commented Jul 22, 2024

NickSneo commented Jul 26, 2024 •

edited

Loading

macfarla left a comment

NickSneo commented Aug 1, 2024

NickSneo commented Aug 12, 2024 •

edited

Loading

macfarla left a comment

pinges left a comment

Tracing private transactions feature #6161

Tracing private transactions feature #6161

Conversation

NickSneo commented Nov 12, 2023

PR description

Fixed Issue(s)

github-actions bot commented Nov 12, 2023 • edited by NickSneo Loading

macfarla commented Nov 13, 2023

NickSneo commented Nov 13, 2023

pinges left a comment

Choose a reason for hiding this comment

NickSneo commented Apr 30, 2024

NickSneo commented May 7, 2024

macfarla commented May 16, 2024

NickSneo commented May 17, 2024

NickSneo commented Jun 10, 2024 • edited Loading

NickSneo commented Jul 3, 2024

pinges left a comment

Choose a reason for hiding this comment

pinges left a comment

Choose a reason for hiding this comment

NickSneo commented Jul 22, 2024

NickSneo commented Jul 26, 2024 • edited Loading

macfarla left a comment

Choose a reason for hiding this comment

NickSneo commented Aug 1, 2024

NickSneo commented Aug 12, 2024 • edited Loading

macfarla left a comment

Choose a reason for hiding this comment

pinges left a comment

Choose a reason for hiding this comment

github-actions bot commented Nov 12, 2023 •

edited by NickSneo

Loading

NickSneo commented Jun 10, 2024 •

edited

Loading

NickSneo commented Jul 26, 2024 •

edited

Loading

NickSneo commented Aug 12, 2024 •

edited

Loading