Forked ChainManager #4593
Forked ChainManager #4593
Conversation
envestcc
requested review from
CoderZhi,
dustinxie,
Liuhaai and
a team
as code owners
| d.mu.Lock() | ||
| defer d.mu.Unlock() | ||
| for _, b := range d.nodes { | ||
| if b.PrevHash() == prevHash && b.Timestamp().Equal(timestamp) { |
There was a problem hiding this comment.
from the logic here, it's possible to have 2 blocks with same prevHash and diff time, is that right?
b1.PrevHash() = 0x1234, b1.Time() = 17:56:32
b2.PrevHash() = 0x1234, b2.Time() = 17.56.35 (3 seconds later)
see comments at L55
There was a problem hiding this comment.
yes, but in practice it should be relatively rare, which requires all 24 delegates to fail.
| // it must be a new tip of any fork, or make a new fork | ||
| prevHash := blk.PrevHash() | ||
| if _, ok := d.leaves[prevHash]; ok { | ||
| delete(d.leaves, prevHash) |
There was a problem hiding this comment.
if trying to add 2 blocks with same prevHash, then only the first will be added successfully, say
b1.PrevHash() = 0x1234, b1.Time() = 17:56:32
b2.PrevHash() = 0x1234, b2.Time() = 17.56.35 (3 seconds later)
only b1 will be added (which deletes d.leaves[0x1234], b2 won't be added into the pool
then situation at L126 won't happend? see comments at L126
There was a problem hiding this comment.
No, b1 and b2 are both included as two forks in your case. It won't go L55, b/c the tip is b1 instead of b1.prev.
| log.L().Debug("remove block from draft pool", log.Hex("hash", ha[:]), zap.Uint64("height", b.Height()), zap.Time("timestamp", b.Timestamp())) | ||
| delete(d.nodes, b.HashBlock()) | ||
| } | ||
| } |
There was a problem hiding this comment.
let's sync offline for this part of logic
| start = blk | ||
| } | ||
| for b := start; b != nil; b = d.nodes[b.PrevHash()] { | ||
| ha := b.HashBlock() | ||
| log.L().Debug("remove block from draft pool", log.Hex("hash", ha[:]), zap.Uint64("height", b.Height()), zap.Time("timestamp", b.Timestamp())) | ||
| delete(d.nodes, b.HashBlock()) | ||
| } | ||
| if !has || blk.HashBlock() == leaf { | ||
| leavesToDelete = append(leavesToDelete, leaf) |
There was a problem hiding this comment.
so just to confirm, for the case we discussed, this will delete A, B, 3, 4, and 1, 2 remain as 2 leaves?
|
Description
Changes for ChainManager:
MintNewBlockmethod and caches the minted blocks.base #4590 and #4581
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Test Configuration:
Checklist: