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

Feature addition for tracing private transactions #5534

Closed
wants to merge 175 commits into from

Conversation

NickSneo
Copy link
Contributor

@NickSneo NickSneo commented Jun 5, 2023

PR description

PR for adding priv_traceTransaction API

Fixed Issue(s)

#5280

@NickSneo NickSneo marked this pull request as draft June 5, 2023 07:17
@github-actions
Copy link

github-actions bot commented Jun 5, 2023

  • 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

@NickSneo NickSneo force-pushed the nicks/issue-5280 branch from 4e0e4b1 to aef0213 Compare June 8, 2023 15:41
@NickSneo NickSneo force-pushed the nicks/issue-5280 branch 2 times, most recently from 2d6e2ae to cfaf6ee Compare July 31, 2023 10:25
@NickSneo NickSneo marked this pull request as ready for review July 31, 2023 10:28
@macfarla
Copy link
Contributor

macfarla commented Aug 2, 2023

@NickSneo for context can you add notes on any testing you've done with this in privacy-enabled networks?

@NickSneo
Copy link
Contributor Author

NickSneo commented Aug 7, 2023

@NickSneo for context can you add notes on any testing you've done with this in privacy-enabled networks?

Hey @macfarla , sorry for the delay in replying
I tested it with privacy enabled network. Created a privacy group, deployed a private contract and did a private transaction. Then called the priv_traceTransaction api using node with access to private tx -

curl -X POST --data '{"jsonrpc": "2.0", "method": "priv_traceTransaction","params": ["4tlg6LYuf3gd2FnFtamTB/J7KxS3YfJOR/Hej4MQS9s=","0x0e660fd71d6229d172de81a7d2ad234034c1b9ad58798b72493a9ede5d2e22df"],"id": 1}' http://127.0.0.1:8545
{
  "jsonrpc" : "2.0",
  "id" : 1,
  "result" : [ {
    "action" : {
      "callType" : "call",
      "from" : "0x7ee801898161f84fba714bdbb4a388ebd49273fd",
      "input" : "0x6057361d00000000000000000000000000000000000000000000000000000000000003e8",
      "to" : "0x13acff278c623b014359eca99e6120515fde42a9",
      "value" : "0x0"
    },
    "blockHash" : "0x361ba4389cdcfb74565bb99e66a4dd53cc6644369c9801e5d4e79bff319b7976",
    "blockNumber" : 2365,
    "result" : {
      "gasUsed" : "0x0",
      "output" : "0x"
    },
    "subtraces" : 0,
    "traceAddress" : [ ],
    "transactionHash" : "0x0e660fd71d6229d172de81a7d2ad234034c1b9ad58798b72493a9ede5d2e22df",
    "type" : "call"
  } ]
}

Then I called the same api from node with no access to private tx -

curl -X POST --data '{"jsonrpc": "2.0", "method": "priv_traceTransaction","params": ["4tlg6LYuf3gd2FnFtamTB/J7KxS3YfJOR/Hej4MQS9s=","0x0e660fd71d6229d172de81a7d2ad234034c1b9ad58798b72493a9ede5d2e22df"],"id": 1}' http://127.0.0.1:8546
{
  "jsonrpc" : "2.0",
  "id" : 1,
  "result" : [ ]
}

Seems the API is working fine. Planning to add the tests for this feature in another PR, since this PR is already very big

@NickSneo NickSneo requested a review from macfarla August 8, 2023 07:55
@NickSneo
Copy link
Contributor Author

Hey @macfarla , can you please review the PR
Thanks!

@NickSneo for context can you add notes on any testing you've done with this in privacy-enabled networks?

Hey @macfarla , sorry for the delay in replying I tested it with privacy enabled network. Created a privacy group, deployed a private contract and did a private transaction. Then called the priv_traceTransaction api using node with access to private tx -

curl -X POST --data '{"jsonrpc": "2.0", "method": "priv_traceTransaction","params": ["4tlg6LYuf3gd2FnFtamTB/J7KxS3YfJOR/Hej4MQS9s=","0x0e660fd71d6229d172de81a7d2ad234034c1b9ad58798b72493a9ede5d2e22df"],"id": 1}' http://127.0.0.1:8545
{
  "jsonrpc" : "2.0",
  "id" : 1,
  "result" : [ {
    "action" : {
      "callType" : "call",
      "from" : "0x7ee801898161f84fba714bdbb4a388ebd49273fd",
      "input" : "0x6057361d00000000000000000000000000000000000000000000000000000000000003e8",
      "to" : "0x13acff278c623b014359eca99e6120515fde42a9",
      "value" : "0x0"
    },
    "blockHash" : "0x361ba4389cdcfb74565bb99e66a4dd53cc6644369c9801e5d4e79bff319b7976",
    "blockNumber" : 2365,
    "result" : {
      "gasUsed" : "0x0",
      "output" : "0x"
    },
    "subtraces" : 0,
    "traceAddress" : [ ],
    "transactionHash" : "0x0e660fd71d6229d172de81a7d2ad234034c1b9ad58798b72493a9ede5d2e22df",
    "type" : "call"
  } ]
}

Then I called the same api from node with no access to private tx -

curl -X POST --data '{"jsonrpc": "2.0", "method": "priv_traceTransaction","params": ["4tlg6LYuf3gd2FnFtamTB/J7KxS3YfJOR/Hej4MQS9s=","0x0e660fd71d6229d172de81a7d2ad234034c1b9ad58798b72493a9ede5d2e22df"],"id": 1}' http://127.0.0.1:8546
{
  "jsonrpc" : "2.0",
  "id" : 1,
  "result" : [ ]
}

Seems the API is working fine. Planning to add the tests for this feature in another PR, since this PR is already very big

@macfarla macfarla requested a review from pinges August 14, 2023 04:30
@NickSneo
Copy link
Contributor Author

Hey @pinges , can you please review this PR?
Thanks!

final String privacyGroupId) {
return (transaction, header, blockchain, transactionProcessor, dataGasPrice) -> {
// if we have no prior updater, it must be the first TX, so use the block's initial state
if (chainedUpdater == null) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

won't chainedUpdater always be null at this point since it doesn't get set until below?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am using the same logic as used in BlockTracer, just added extra logic for PrivateTransaction type and private world states.
Also I guess it will be null for first transaction only, once updater is set it will reuse it

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Signed-off-by: Nischal Sharma <nischal@web3labs.com>
Signed-off-by: Nischal Sharma <nischal@web3labs.com>
Signed-off-by: Nischal Sharma <nischal@web3labs.com>
Signed-off-by: Nischal Sharma <nischal@web3labs.com>
Signed-off-by: Nischal Sharma <nischal@web3labs.com>
Signed-off-by: Nischal Sharma <nischal@web3labs.com>
Signed-off-by: Nischal Sharma <nischal@web3labs.com>
Signed-off-by: Nischal Sharma <nischal@web3labs.com>
Signed-off-by: Nischal Sharma <nischal@web3labs.com>
Signed-off-by: Nischal Sharma <nischal@web3labs.com>
Signed-off-by: Nischal Sharma <nischal@web3labs.com>
gfukushima and others added 28 commits November 12, 2023 08:42
* Accept input or data as payload for RPC calls

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

* Add json new rpc valid and invalid request to test the changes

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

* Change JsonCallParameter signature to avoid duplicating constructor

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

* Add changelog

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

---------

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>
* Fix some typos
* Update plugin version

Signed-off-by: GoodDaisy <90915921+GoodDaisy@users.noreply.github.com>

---------

Signed-off-by: GoodDaisy <90915921+GoodDaisy@users.noreply.github.com>
* Correct reference test blobgas calculation

Fix tpyo that resulted in an NPE in t8n blob gas calculations.

Signed-off-by: Danno Ferrin <danno.ferrin@swirldslabs.com>

* changelog

Signed-off-by: Danno Ferrin <danno.ferrin@swirldslabs.com>

* spotless

Signed-off-by: Danno Ferrin <danno.ferrin@swirldslabs.com>

---------

Signed-off-by: Danno Ferrin <danno.ferrin@swirldslabs.com>
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
…perledger#6067)

* Don't put NONCE_TOO_LOW transactions into the invalid nonce cache

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>

* Update unit tests

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>

* Use list of errors to ignore

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>

---------

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>
Signed-off-by: Matt Whitehead <matthew1001@gmail.com>
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Co-authored-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Introduce a new Journaled World State Updater. Within a transaction it
keeps one copy of account and storage state, restoring previous 
revisions on reverts and exceptional halts. This updater only supports 
post-merge semantics with regard to empty accounts, namely that they do
not exist in world state.

Adds an EvmConfiguration option for stacked vs journaled updater, and
wire it in where needed. The staked updater is default mode, which is
the current behavior prior to this patch.

Signed-off-by: Danno Ferrin <danno.ferrin@swirldslabs.com>
…imal instead of Long (hyperledger#6099)

Signed-off-by: Gabriel-Trintinalia <gabriel.trintinalia@consensys.net>
Signed-off-by: Gabriel-Trintinalia <gabriel.trintinalia@consensys.net>
…dger#6083)

Signed-off-by: Gabriel-Trintinalia <gabriel.trintinalia@consensys.net>
There was a slight problem on the bonsai side because all account reads did not go through a single method. One of the two add the account to the cache but the other did not. This had two consequences:

Less good performance because certain accounts had to be read several times and also all account reads were not marked in the trielog. This will fix both problems.

Signed-off-by: Karim TAAM <karim.t2am@gmail.com>
v13 of the official Ethereum Reference Tests.

Signed-off-by: Danno Ferrin <danno.ferrin@swirldslabs.com>
* clean up

Signed-off-by: stefan.pingel@consensys.net <stefan.pingel@consensys.net>
Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>
Update the GraphQL and JSON-RPC endpoints to provide `yParity` instead
of `v` for non-legacy transactions.
Update the JSON-RPC tests to use the Hive data. Add tests for Shanghai
and Cancun Blocks.

Signed-off-by: Danno Ferrin <danno.ferrin@swirldslabs.com>
Signed-off-by: stefan.pingel@consensys.net <stefan.pingel@consensys.net>
* Revert "Log missing chain head as warning, not trace (hyperledger#6127)"
* leave chain head at warning, just revert the discard warning

Signed-off-by: garyschulte <garyschulte@gmail.com>
* Reverse added order and sequence number

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>

* Remove extraneous 'addedAt' check

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>

---------

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>
…configured (hyperledger#6079)


Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com>
Signed-off-by: Jason Frame <jason.frame@consensys.net>
…ution (hyperledger#6102)

* Delete leftPad when capturing the stack before and after the execution
* Still use leftPad when displaying the stack in the output (ex. for debug_traceTransaction)
* Fix integration test
* Use StringBuilder to left pad the hex representation of a 32 bytes

Signed-off-by: Ameziane H <ameziane.hamlat@consensys.net>
Fix the flakeiness in EthGetTransactionByHashTest as well as some other
sonar identified cleanup.

Signed-off-by: Danno Ferrin <danno.ferrin@swirldslabs.com>
hyperledger#6146)

* Apply the same reverse sort order as hyperledger#6106 but to the base fee sorter

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>

* Fix unit tests

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>

* Update eviction unit tests to expect highest-sequence TXs be evicted first

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>

* Update change log

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>

* Spotless fixes

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>

---------

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>
Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>
Signed-off-by: Justin Florentine <justin+github@florentine.us>
@NickSneo
Copy link
Contributor Author

Closing this PR.
Created a new PR for this feature - #6161

@NickSneo NickSneo closed this Nov 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.