[AV-1026] vm: implement "AppGossip" #8
Conversation
|
Next, we'll need to implement the |
vm/vm.go
Outdated
|
|
||
| log.Debug("AppGossip %d transactions are being added to mempool", len(txs)) | ||
| for _, tx := range txs { | ||
| switch vm.mempool.Add(tx) { |
There was a problem hiding this comment.
You'll also want to verify these txs before insertion (see logic in SubmitTx)
There was a problem hiding this comment.
You may just want to abstract the inside of that func so you only call notifyBuildBlock after all txs are added to the mempool.
There was a problem hiding this comment.
SubmitTx may also handle the re-gossiping for you (as you'll want to continue propagating tx to other nodes)
vm/vm.go
Outdated
|
|
||
| // next iteration should be gossip | ||
| // after waiting for this node to build a block | ||
| buildBlk = false |
There was a problem hiding this comment.
I think we should do something more sophisticated here.
There was a problem hiding this comment.
Take a look at BlockBuilder for some inspiration: https://github.com/ava-labs/coreth-internal/blob/97cd3d0643c3a30cb3b0f1b4557151c254957069/plugin/evm/block_builder.go
vm: batch transactions in "Submit" mempool: prevent races in mempool Signed-off-by: Gyuho Lee <[email protected]>
vm/vm.go
Outdated
| // wait for this node to build a block | ||
| // rather than trigger gossip immediately | ||
| select { | ||
| case <-vm.fromEngine: |
There was a problem hiding this comment.
There should be a timeout here. Maybe something like 1s?
In an ideal world, we have the option to attempt to build a block synchronously (and if can't it says we won't).
There was a problem hiding this comment.
If we don't build a block, we should still gossip.
There was a problem hiding this comment.
the real complication here is that there could be stale messages in this channel after we pass here so that on the next read, we think a block was just triggered when there wasn't.
There was a problem hiding this comment.
What you may need to do is just have an atomic var that you reset before calling NotifyBlockReady
There was a problem hiding this comment.
Leaving it as TODO. Looks like, unless we add request ID for each common.PendingTxs, there's no way to confirm the engine events.
|
|
||
| // Triggers "AppGossip" on the pending transactions in the mempool. | ||
| // "force" is true to re-gossip whether recently gossiped or not | ||
| func (vm *VM) GossipTxs(force bool) error { |
There was a problem hiding this comment.
If you implement forced gossip, you should also add the regossip ticker: https://github.com/ava-labs/coreth-internal/blob/97cd3d0643c3a30cb3b0f1b4557151c254957069/plugin/evm/network.go#L267-L277
vm/network.go
Outdated
| // submit incoming gossip | ||
| log.Debug("AppGossip transactions are being submitted", "txs", len(txs)) | ||
| if err := vm.Submit(txs...); err != nil { | ||
| log.Warn( |
There was a problem hiding this comment.
You should only do log.Trace on incoming gossip verification.
There was a problem hiding this comment.
Otherwise, easy to trigger you to write a shit ton of logs from a malicious peer
There was a problem hiding this comment.
No Trace level logging in the one used here. So, switched to Debug level.
Signed-off-by: Gyuho Lee <[email protected]>
Signed-off-by: Gyuho Lee <[email protected]>
2 participants
https://github.com/ava-labs/coreth-internal/blob/master/plugin/evm/network.go
https://github.com/ava-labs/avalanchego/blob/dev/vms/platformvm/network.go