Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Stackoverflow fix #1742

Merged
merged 10 commits into from
Jul 28, 2016
Merged

Stackoverflow fix #1742

merged 10 commits into from
Jul 28, 2016

Conversation

debris
Copy link
Collaborator

@debris debris commented Jul 27, 2016

fixes #1704

@debris debris added the A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. label Jul 27, 2016
@debris debris added A0-pleasereview 🤓 Pull request needs code review. and removed A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. labels Jul 28, 2016
@gavofyork
Copy link
Contributor

this alters the user-visible JSONRPC format; can we do a follow-up PR prior to 1.3 to reinstate the original trace format?

@debris
Copy link
Collaborator Author

debris commented Jul 28, 2016

Only trace_call, trace_rawTransaction and trace_replayTransaciton are changed and there is no way of preserving the old format cause serialization of data will overflow the stack.

rawTransaction and replayTransaction were merged 2 days ago, so they were not yet in release version. The only broken method is trace_call, but afaik it's not used by exchanges/block explorers.

@coveralls
Copy link

coveralls commented Jul 28, 2016

Coverage Status

Coverage decreased (-0.2%) to 86.689% when pulling 955dc53 on stackoverflow_fix into 6b1e722 on master.

@gavofyork
Copy link
Contributor

gavofyork commented Jul 28, 2016

it is used by some of my ui, and in general the heirarchical JSON more closely represents the underlying information. i'd prefer not to cut any corners on standards just because serde's model is inadequate. ultimately, these should be adopted across clients and it's hard to make a case for a substandard API/format on the argument of "a library of one of the implementation makes this format easier".

@debris
Copy link
Collaborator Author

debris commented Jul 28, 2016

it is used by some of my ui, and in general the heirarchical JSON more closely represents the underlying information

I agree, but this model does not work now. It overflows very often when serializing to rlp / serializing to json

i'd prefer not to cut any corners on standards just because serde's model is inadequate.

We already have one format of returning traces on which we agreed on few months ago. This format is already used be exchanges / block explorers. I don't see a reason to have another one

@gavofyork
Copy link
Contributor

gavofyork commented Jul 28, 2016

they fulfil two different needs though. the format to which you linked must handle the case of returning a collection of individual messages/creates/suicides from across many transactions.

when tracing a single individual transaction, that is not needed; rather a hierarchical/recursive format makes more sense since it directly reflects the structure of the call graph.

this model does not work now

i repeat: i'd prefer not to cut any corners on standards just because serde's model is inadequate.

@arkpar
Copy link
Collaborator

arkpar commented Jul 28, 2016

I'd prefer to have a single output format for all types of queries. We might also want to filter individual tx traces by address in the future for example.

@gavofyork
Copy link
Contributor

gavofyork commented Jul 28, 2016

we have different outputs for different RPC calls - it's all about fitting the format to the structure of the data. this is no different.

@gavofyork gavofyork added A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jul 28, 2016
@gavofyork gavofyork merged commit 9746b94 into master Jul 28, 2016
@gavofyork gavofyork deleted the stackoverflow_fix branch July 28, 2016 18:31
@debris debris mentioned this pull request Jul 29, 2016
arkpar pushed a commit that referenced this pull request Jul 29, 2016
* executive tracer builds flat traces without intermediate struct

* temporarilt commented out tests for traces

* fixed new way of building trace address

* fixed new way of building trace address

* updating state tests with flat tracing in progress

* fixed flat tracing tests

* fixed compiling ethcore-rpc with new flat traces

* removed warnings from ethcore module

* remove unused data structures
arkpar pushed a commit that referenced this pull request Jul 29, 2016
* executive tracer builds flat traces without intermediate struct

* temporarilt commented out tests for traces

* fixed new way of building trace address

* fixed new way of building trace address

* updating state tests with flat tracing in progress

* fixed flat tracing tests

* fixed compiling ethcore-rpc with new flat traces

* removed warnings from ethcore module

* remove unused data structures
gavofyork pushed a commit that referenced this pull request Jul 30, 2016
* Don't try to sync to ancient blocks

* Parallel block body download

* Fixed reading chunked EIP8 handshake (#1712)

* Fixed reading chunked EIP8 handshake

* Added missing break

* Disconnect peers on a fork

* Updated json-ipc-server

* Combine mining queue and enabled into single locked datum (#1749)

* Combine mining queue and enabled into single locked datum

Additional tracing.

* Fix bug uncovered by test.

* Fix typo

* Remove unneeded log initialisation in test.

* fix failing test (#1756)

* Fixed test

* Suicides tracing (#1688)

* tracing suicide

* fixed #1635

* fixed typo

* Stackoverflow #1686 (#1698)

* flat trace serialization

* tracing finds transaction which creates contract

* flatten traces before inserting them to the db

* Trace other types of calls (#1727)

* Trace through DELEGATECALL and CALLCODE

Add them to the JSON output and RLP database store.

* Fix tests.

* Fix all tests.

* Fix one more test.

* filtering transactions toAddress includes contract creation (#1697)

* tracing finds transaction which creates contract

* comma cleanup

Remove when following `}`s, add to final entries.

* Various improvements to tracing & diagnostics. (#1707)

* Various improvements to tracing & diagnostics.

- Manage possibility of `Account` not having code for `PodAccount`
- New RPC: `trace_sendRawTransaction`
- See raw transaction dump when inspecting over RPC

* Fix test

* Remove one of the dupe error messages

* Remove unneeded `&`s

* Reformat and extremely minor optimisation

* Minor optimisation

* Remove unneeded let

* Fix tests.

* Additional fix.

* Minor rename.

* Bowing to the pressure.

* Stackoverflow fix (#1742)

* executive tracer builds flat traces without intermediate struct

* temporarilt commented out tests for traces

* fixed new way of building trace address

* fixed new way of building trace address

* updating state tests with flat tracing in progress

* fixed flat tracing tests

* fixed compiling ethcore-rpc with new flat traces

* removed warnings from ethcore module

* remove unused data structures
gavofyork pushed a commit that referenced this pull request Jul 30, 2016
* Suicides tracing (#1688)

* tracing suicide

* fixed #1635

* fixed typo

* Stackoverflow #1686 (#1698)

* flat trace serialization

* tracing finds transaction which creates contract

* flatten traces before inserting them to the db

* Trace other types of calls (#1727)

* Trace through DELEGATECALL and CALLCODE

Add them to the JSON output and RLP database store.

* Fix tests.

* Fix all tests.

* Fix one more test.

* filtering transactions toAddress includes contract creation (#1697)

* tracing finds transaction which creates contract

* comma cleanup

Remove when following `}`s, add to final entries.

* Various improvements to tracing & diagnostics. (#1707)

* Various improvements to tracing & diagnostics.

- Manage possibility of `Account` not having code for `PodAccount`
- New RPC: `trace_sendRawTransaction`
- See raw transaction dump when inspecting over RPC

* Fix test

* Remove one of the dupe error messages

* Remove unneeded `&`s

* Reformat and extremely minor optimisation

* Minor optimisation

* Remove unneeded let

* Fix tests.

* Additional fix.

* Minor rename.

[ci:skip]

* Bowing to the pressure.

* Stackoverflow fix (#1742)

* executive tracer builds flat traces without intermediate struct

* temporarilt commented out tests for traces

* fixed new way of building trace address

* fixed new way of building trace address

* updating state tests with flat tracing in progress

* fixed flat tracing tests

* fixed compiling ethcore-rpc with new flat traces

* removed warnings from ethcore module

* remove unused data structures
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

thread '<main>' panicked at 'called when calling 'trace_transaction'
4 participants