Skip to content

Commit 7c2add0

Browse files
committed
dbft: allow to apply Eth-based block verification for unsigned block
To be able to properly verify unsigned block after PrepareRequest receival in WithVerifyBlock dBFT callback we need to ensure that (*DBFT).Author can handle such unsugned block. Starting from the previous commit, we define block's Coinbase as the Primary address for the dBFT round when the block was accepted. And thus, return proper block author for proper verification. The problem itself is described in #87 (comment). Also ref. #51. Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
1 parent 8c80a73 commit 7c2add0

File tree

1 file changed

+22
-6
lines changed

1 file changed

+22
-6
lines changed

consensus/dbft/dbft.go

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -635,13 +635,29 @@ func (c *DBFT) postBlock(b *types.Block) {
635635
}
636636

637637
// Author implements consensus.Engine, returning the Ethereum address recovered
638-
// from the signature in the header's extra-data section.
638+
// from the header's Coinbase.
639639
func (c *DBFT) Author(header *types.Header) (common.Address, error) {
640-
vals, _, err := getSignersAndSigs(c.config, header.Extra)
641-
if err != nil {
642-
return common.Address{}, fmt.Errorf("failed to retrieve validators addresses and signatures from header: %w", err)
643-
}
644-
return vals[header.Primary()], nil
640+
// TODO: we must agree on this with @roman-khimov and @chenquanyu. Block author
641+
// must be defined by the moment of WithVerifyBlock dBFT callback call, thus,
642+
// can't depend on changeable block's Extra field. At the same time, block author
643+
// must correspond to miner.etherbase setting used to run node for proper block
644+
// verification and reward distribution. From my POW, we have two options to
645+
// provide these constraints:
646+
// 1. For each consensus node, in the node config enforce miner.etherbase be
647+
// a corresponding consensus node address, make different miner.etherbase
648+
// no-op for DBFT engine. Then, miner will submit task with coinbase (that's
649+
// the same as miner.coinbase) to dBFT and dBFT will manage coinbase internally
650+
// and set it to the Primary of the current dBFT round. WithNewBlockFromContext
651+
// will provide this. Given this fact, Coinbase is always the address of Primary
652+
// that was the block's author or the accepted block at the current height. And
653+
// then Coinbase then can be used for reward calculations.
654+
// 2. If Coinbase is expected to have another meaning (i.e. it's expected to be
655+
// some side address), then we must share the rules of Coinbase detection between
656+
// consensus nodes. In this case we need to decide what's the desired meaning of Coinbase.
657+
//
658+
// Currently option 1 is implemented, but without enforced miner.etherbase check. Please,
659+
// write your thoughts on this.
660+
return header.Coinbase, nil
645661
}
646662

647663
// VerifyHeader checks whether a header conforms to the consensus rules.

0 commit comments

Comments
 (0)