Skip to content

Commit

Permalink
fix: miner does not mine its inturn block (#268)
Browse files Browse the repository at this point in the history
We have an issue that a miner A is inturn for block 10. At block 9, due to a
node is down, miner A mines the out of turn block 9 and broadcast to the
network. As a result, miner A cannot mine block 10 due to recently signed rule.
However, at block 9, there is another miner B that mines block 9' and this block
is accepted by most of nodes in the network. Then based on block 9', block 10 is
mined by miner C and there is a slash transaction for miner A because on network
view block 9 is mined by miner B not miner A. Miner A receives the chain 9' -
10 and reorgs to that chain. In theory, miner can mine the block 10' which
creates a canonical has greater difficulty. This drops the block 10 and the
slash transaction. However, currently, miner tries to mine block 11 instead of
reproducing new block 10. This leads to miner is slashed even though the node is
running well.

To fix this issue, when mining, we try to go backward on the canonical chain to
find whether we can produce a better chain. If we cannot find way to create a
better chain, we start mining based on the current head block same as current
implementation.

Fixes: #266
  • Loading branch information
minh-bq authored Apr 26, 2023
1 parent 1bf662a commit 94bcf95
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 1 deletion.
4 changes: 4 additions & 0 deletions consensus/consortium/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,10 @@ func (c *Consortium) IsSystemContract(to *common.Address) bool {
return c.v2.IsSystemContract(to)
}

func (c *Consortium) GetBestParentBlock(chain *core.BlockChain) (*types.Block, bool) {
return c.v2.GetBestParentBlock(chain)
}

// HandleSubmitBlockReward determines if the transaction is submitBlockReward
// transaction with non-zero msg.value and fixes up the statedb.
// This function bases on the fact that submitBlockReward is the only system
Expand Down
41 changes: 41 additions & 0 deletions consensus/consortium/v2/consortium.go
Original file line number Diff line number Diff line change
Expand Up @@ -912,6 +912,47 @@ func (c *Consortium) readSigner() (common.Address, consortiumCommon.SignerFn, co
return c.val, c.signFn, c.signTxFn
}

// GetBestParentBlock goes backward in the canonical chain to find if the miner can
// create a chain which has more difficulty than current chain. In case the miner
// cannot create a better chain, this function returns the head block of current
// canonical chain.
func (c *Consortium) GetBestParentBlock(chain *core.BlockChain) (*types.Block, bool) {
signer, _, _ := c.readSigner()

currentBlock := chain.CurrentBlock()
block := currentBlock
prevBlock := chain.GetBlockByHash(block.ParentHash())
diffculty := block.Difficulty().Int64()
for diffculty < diffInTurn.Int64() {
snap, err := c.snapshot(chain, block.NumberU64()-1, block.ParentHash(), nil)
if err != nil {
return currentBlock, false
}
// Miner can create an inturn block which helps the chain to have
// greater diffculty
if snap.supposeValidator() == signer {
inRecent := false
for seen, recent := range snap.Recents {
if recent == signer {
if limit := uint64(len(snap.Validators)/2 + 1); seen > snap.Number+1-limit {
inRecent = true
break
}
}
}
if !inRecent {
return prevBlock, true
}
}

block = prevBlock
prevBlock = chain.GetBlockByHash(block.ParentHash())
diffculty += block.Difficulty().Int64()
}

return currentBlock, false
}

// ecrecover extracts the Ronin account address from a signed header.
func ecrecover(header *types.Header, sigcache *lru.ARCCache, chainId *big.Int) (common.Address, error) {
// If the signature's already cached, return that
Expand Down
3 changes: 3 additions & 0 deletions eth/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -462,6 +462,9 @@ func (s *Ethereum) shouldPreserve(block *types.Block) bool {
if _, ok := s.engine.(*clique.Clique); ok {
return false
}
if _, ok := s.engine.(*consortium.Consortium); ok {
return false
}
return s.isLocalBlock(block)
}

Expand Down
20 changes: 19 additions & 1 deletion miner/worker.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
mapset "github.com/deckarep/golang-set"
"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/consensus"
"github.com/ethereum/go-ethereum/consensus/consortium"
"github.com/ethereum/go-ethereum/consensus/misc"
"github.com/ethereum/go-ethereum/core"
"github.com/ethereum/go-ethereum/core/state"
Expand Down Expand Up @@ -1013,7 +1014,16 @@ func (w *worker) commitNewWork(interrupt *int32, noempty bool, timestamp int64)
defer w.mu.RUnlock()

tstart := time.Now()
parent := w.chain.CurrentBlock()

var (
parent *types.Block
fastCommit bool
)
if consortiumEngine, ok := w.engine.(*consortium.Consortium); ok {
parent, fastCommit = consortiumEngine.GetBestParentBlock(w.chain)
} else {
parent = w.chain.CurrentBlock()
}

if parent.Time() >= uint64(timestamp) {
timestamp = int64(parent.Time() + 1)
Expand Down Expand Up @@ -1096,6 +1106,14 @@ func (w *worker) commitNewWork(interrupt *int32, noempty bool, timestamp int64)
commitUncles(w.localUncles)
commitUncles(w.remoteUncles)

// Fast commit an empty block which has block number smaller
// than current block in canonical chain but have greater
// difficulty
if fastCommit {
w.commit(uncles, nil, true, tstart)
return
}

// Create an empty block based on temporary copied state for
// sealing in advance without waiting block execution finished.
if !noempty && atomic.LoadUint32(&w.noempty) == 0 {
Expand Down

0 comments on commit 94bcf95

Please sign in to comment.