fix(txSkip): address collisions #1510
Conversation
Signed-off-by: F Bojarski <ceciestunepoubelle@protonmail.ch>
Signed-off-by: F Bojarski <ceciestunepoubelle@protonmail.ch>
Signed-off-by: F Bojarski <ceciestunepoubelle@protonmail.ch>
There was a problem hiding this comment.
no real change in this file, just added final, moved some function to TransactionProcessingMetadataand factorize the last transaction fragment.
…ss-collision-shenanigans-into-account
…ss-collision-shenanigans-into-account
testing/src/main/java/net/consensys/linea/testing/ToyExecutionEnvironmentV2.java
Outdated
Show resolved
Hide resolved
Signed-off-by: F Bojarski <ceciestunepoubelle@protonmail.ch>
…ss-collision-shenanigans-into-account
| } | ||
|
|
||
| @Test | ||
| void senderIsCoinbaseIsReceiver() { |
There was a problem hiding this comment.
One test to rule them all 👑
“It is a strange fate that we should suffer so much fear and doubt over so small a thing … such a little thing.”
| .zkTracerValidator(zkTracer -> {}) | ||
| .build() | ||
| .run(); | ||
| } |
There was a problem hiding this comment.
Does this test succeed ? We had a regression in Besu with @ahamlat / @Gabriel-Trintinalia / @lu-pinto's fix being undone in a recent commit, which may make this test fail now.
| DomSubStampsSubFragment.standardDomSubStamps(hub.stamp(), 2)); | ||
|
|
||
| this.addFragments(firstAccountFragment, secondAccountFragment, thirdAccountFragment); | ||
| } |
There was a problem hiding this comment.
We will really need account consistency constraints to test this properly 😅.
| AccountSnapshot.canonical(hub, world, recipientAccountSnapshotBefore.address()); | ||
|
|
||
| final AccountSnapshot coinbaseAccountSnapshotAfter = | ||
| AccountSnapshot.canonical(hub, world, coinbaseAccountSnapshotBefore.address()); |
There was a problem hiding this comment.
I'm noticing that in the old version we were copying over the warmths. I don't have the spec in mind, and I doubt that it fundamentally matters. But the question remains: is the fact that there is no mention of warmths an oversight ?
The spec says that one should keep warmths for TX_SKIP as they are. I wonder what that would mean e.g. if
- sender is a precompile (can we even test this ? Or would we need to produce a signature for the (unknown) private key of a precompile) ?
- coinbase is a precompile (those are warm by default)
N.B. Recipients may not be precompiles.
There was a problem hiding this comment.
I don't think IRL it's possible to have sender / coinbase that is a precompile ...
In the spec, we only constrain the warmth to remains constant, thus it's better to be hot -> hot as it could be warmed (being on the accessList , or so). So when taking teh snapshot, better to turn on the warmth all the time.
| .accountFragment() | ||
| .make( | ||
| senderSnapshotBeforeFinalization, | ||
| coinbaseSnapshotAfterFinalization, |
There was a problem hiding this comment.
This should indeed work
|
Also if we don't have them yet we should add tests for TX_SKIP with trivial deployments. We can play the same games of
|
letypequividelespoubelles
deleted the
1018-txskippedsection-doesnt-take-address-collision-shenanigans-into-account
branch
No description provided.