feat: txgossip package
#26
Conversation
…l be much easier for everyone
Co-authored-by: Stephen Buttolph <[email protected]> Signed-off-by: Arran Schlosberg <[email protected]>
Co-authored-by: Stephen Buttolph <[email protected]> Signed-off-by: Arran Schlosberg <[email protected]>
Co-authored-by: Stephen Buttolph <[email protected]> Signed-off-by: Arran Schlosberg <[email protected]>
Co-authored-by: Stephen Buttolph <[email protected]> Signed-off-by: Arran Schlosberg <[email protected]>
Co-authored-by: Stephen Buttolph <[email protected]> Signed-off-by: Arran Schlosberg <[email protected]>
Co-authored-by: Stephen Buttolph <[email protected]> Signed-off-by: Arran Schlosberg <[email protected]>
StephenButtolph
left a comment
StephenButtolph
left a comment
There was a problem hiding this comment.
will continue reviewing tomorrow
txgossip/txgossip.go
Outdated
| // to release resources. | ||
| func NewSet(logger logging.Logger, pool *txpool.TxPool, bloom *gossip.BloomFilter, targetBloomElements int) (*Set, func() error) { | ||
| txs := make(chan core.NewTxsEvent) | ||
| sub := pool.SubscribeTransactions(txs, false) |
There was a problem hiding this comment.
It seems like this only receives txs upon mempool reorgs. I would have thought that the bloom filter should update as soon as we add the tx to the pool to avoid unnecessary gossip
There was a problem hiding this comment.
I've demonstrated in TestTxPoolPopulatesBloomFilter() that it will send new txs as well and that the filter receives them.
There was a problem hiding this comment.
Sorry, my original comment wasn't questioning whether or not the txs would get added to the bloom filter eventually. My comment was questioning whether or not it is smart to wait until a mempool reorg happens to populate the bloom filter.
In all of our current mempool implementations, we include the transactions into the bloom filter prior to returning from Add.
| func (bc *blockchain) StateAt(root common.Hash) (*state.StateDB, error) { | ||
| return state.New(root, bc.exec.StateCache(), nil) | ||
| } |
There was a problem hiding this comment.
So, with this PR the mempool's validity checking is based on the settled state right?
There was a problem hiding this comment.
Intent is currently undefined. Sorry, I should have been clearer in what I meant in the PR description (i.e. I'm kicking that can down the road entirely):
Any specific rules for when to reject a tx from the mempool will likely require some libevm mods so can follow later.
The p2p architecture and gossip was all so new to me that I found this PR relatively difficult, so wanted to focus on learning it and getting the wiring correct. I'll concentrate on things like validity when we have the VM to orchestrate all the moving parts.
Introduces an integration point between
txpool.TxPool,saexec.Executorandgossip.Set, with a simple transaction-prioritisation mechanism. Any specific rules for when to reject a tx from the mempool will likely require somelibevmmods so can follow later.The consensus worst-case validation will still need to track "in-flight" txs as the
LegacyPoolwill only remove them once included. An enqueueing subscription mechanism is added for this purpose.