-
Notifications
You must be signed in to change notification settings - Fork 1.7k
From/To in transaction traces seem to be wrong for delegatecall #7166
Comments
Might be related. #6087 |
I confirm that parity traces was always behaving like this (incorrectly). It is not entirely wrong though. It copies |
But the problem is that Parity displays the context 2 level up, not the correct one. For this particular transaction, we have
So the |
Ah wait, I see what you're saying. The context is Then again, you may again be right in that the Parity trace currently cannot discern the actual contract being delegatecalled. Interesting case :) |
* Update .gitlab-ci.yml fix cache:key * Fixed delegatecall's from/to (#7568) * Fixed delegatecall's from/to, closes #7166 * added tests for delegatecall traces, #7167 * Fix Temporarily Invalid blocks handling (#7613) * Handle temporarily invalid blocks in sync. * Fix tests. * Improve handling of RocksDB corruption (#7630) * kvdb-rocksdb: update rust-rocksdb version * kvdb-rocksdb: mark corruptions and attempt repair on db open * kvdb-rocksdb: better corruption detection on open * kvdb-rocksdb: add corruption_file_name const * kvdb-rocksdb: rename mark_corruption to check_for_corruption
* Improve handling of RocksDB corruption (#7630) * kvdb-rocksdb: update rust-rocksdb version * kvdb-rocksdb: mark corruptions and attempt repair on db open * kvdb-rocksdb: better corruption detection on open * kvdb-rocksdb: add corruption_file_name const * kvdb-rocksdb: rename mark_corruption to check_for_corruption * Hardening of CSP (#7621) * Fixed delegatecall's from/to (#7568) * Fixed delegatecall's from/to, closes #7166 * added tests for delegatecall traces, #7167 * Light client RPCs (#7603) * Implement registrar. * Implement eth_getCode * Don't wait for providers. * Don't wait for providers. * Fix linting and wasm tests. * Problem: AttachedProtocols don't get registered (#7610) I was investigating issues I am having with Whisper support. I've enabled Whisper on a custom test network and inserted traces into Whisper handler implementation (Network<T> and NetworkProtocolHandler for Network<T>) and I noticed that the handler was never invoked. After further research on this matter, I found out that AttachedProtocol's register function does nothing: https://github.com/paritytech/parity/blob/master/sync/src/api.rs#L172 but there was an implementation originally: 99075ad#diff-5212acb6bcea60e9804ba7b50f6fe6ec and it did the actual expected logic of registering the protocol in the NetworkService. However, as of 16d84f8#diff-5212acb6bcea60e9804ba7b50f6fe6ec ("finished removing ipc") this implementation is gone and only the no-op function is left. Which leads me to a conclusion that in fact Whisper's handler never gets registered in the service and therefore two nodes won't communicate using it. Solution: Resurrect original non-empty `AttachedProtocols.register` implementation Resolves #7566 * Fix Temporarily Invalid blocks handling (#7613) * Handle temporarily invalid blocks in sync. * Fix tests.
Hi. I'm not able to discern whether or not anything changed in Parity's traces based on the above discussion. Was there an error in Parity's traces? If yes, does this mean that traces from blocks prior to this fix are now different than they were with earlier version. Saying it a different way: if I cached some traces from old versions of Partiy, should I clear my cache? |
Yes, there was.
Yes, you should |
@debris |
Led here from the release notes for 1.10.0 "We fixed DELEGATECALL" imo is an insanely unhelpful release note - it points here where no definitive conclusion was documented as to exactly what the issue was. Nowhere (apart from the code) is it explained what has changed and why. I think this is pretty important in the context of tjayrush's comment. 3rd parties rely on responses from Parity. They are being changed. There data will now be inconsistent without clear explanation.. |
I'm currently reworking the transaction tracing in go-ethereum, and I've seen some discrepancies between our results and the Parity results from EtherScan. After a quick look, it seems to me that Parity's traces are wrong, but please correct me if I'm wrong.
A sample transaction is https://ropsten.etherscan.io/tx/0x427dfdbe8077d2901a51767672db3d4c37b7b7e4814263019dab099efa8e3e5e
Parity's trace according to etherscan is
The trace produced by go-ethereum is:
The difference between the two traces are in the DELEGATECALL
from
andto
fields. Both Parity and go-ethereum report the the DELEGATECALL 2 nestings deep (tx -> CALL -> DELEGATECALL
). Parity however reports the DELEGATECALL'sfrom
field the same as the main transaction'sto
field (0x269296dddce321a6bcbaa2f0181127593d732cba
). IMHO this is wrong, since we switched context in the intermediate CALL from the0x269296dddce321a6bcbaa2f0181127593d732cba
contract to the0x13204f5d64c28326fd7bd05fd4ea855302d7f2ff
contract.The text was updated successfully, but these errors were encountered: