Check bundle atomicity #96
Conversation
| return nil | ||
| } | ||
|
|
||
| func extractBundleTxDataFromBundles(bundles []types.SimulatedBundle, result map[common.Hash][]bundleTxData) { |
There was a problem hiding this comment.
Minor suggestion: I'd expect result as an outparam to be a pointer to the map because I never know if the map is a reference under the copied value
There was a problem hiding this comment.
I agree but pointer to map does not support m[key] operation so you would still need to do (*m)[key] below.
|
|
||
| // we allow gaps between txs in the bundle, | ||
| // but txs must be in the right order | ||
| if txInclusion.index < currentBlockTx { |
| return nil, nil, err | ||
| } | ||
|
|
||
| err = VerifyBundlesAtomicity(work, blockBundles, allBundles, usedSbundles, mempoolTxHashes) |
There was a problem hiding this comment.
Should we consider putting this behind a flag maybe? I think we always want to run with this check, but who knows
There was a problem hiding this comment.
I added a micro benchmark and it claims to run in 0.25ms on my machine. So it looks like it could be safely be here without flag.
There was a problem hiding this comment.
Your 64-core machine? 😅
With efficient revert and other performance optimizations block build time will go lower. We also want to resubmit more often because it enables more transactions to be included
I updated benchmark for worst-case n=1000 because I sometimes saw 300+ bundles. I could add flag to this branch maybe.
BenchmarkVerifyBundlesAtomicity-12 1272 904799 ns/op
PASS
ok github.com/ethereum/go-ethereum/miner 17.514s
There was a problem hiding this comment.
its non parallel
and in your case its still 1 ms
There was a problem hiding this comment.
btw you don't need to add 1000 committed bundles. even 100 is way to big for committed stuff.
but for used bundles its more correct
Wazzymandias
force-pushed
the
sanity_check_bundles
branch
2 times, most recently
from
804defb to
17d7dbc
Compare
dvush
force-pushed
the
sanity_check_bundles
branch
from
0f91091 to
28d9029
Compare
3 participants
📝 Summary
Sanity check that all bundles that were included in the block are included preserving promised invariant.
CONTRIBUTING.md