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