Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IBFT: Ensure that Committed Seal is not zero value #1118

Merged
merged 14 commits into from
Sep 30, 2021

Conversation

vdamle
Copy link
Contributor

@vdamle vdamle commented Jan 28, 2021

When a commit message is processed, if the Proposal is missing due to
some reason, we should not generate a zero value Committed Seal. Such
an incorrect seal causes BAD BLOCK failure with an invalid signature
error during operations such as Block import.

Details

For an IBFT chain where we gathered a backup using geth export, we hit a BAD BLOCK: invalid signature error running import on the chain data. Upon further investigation, I found that the invalid signature error was because one of the Committed Seals was a Zero address. Looking at the code, it appears that this can happen when a Commit message without a proposal is finalized (I haven't been able to pin point how this occurs, but this appears the only case where the committed seal is set to a zero value)

  • We still have the origin chain running, so I was able to check block info:
instance: Geth/v1.8.18-stable-4bed8ddd(quorum-v2.5.0)/linux-amd64/go1.10.1
coinbase: 0xe38cdc5113361c0a5baa106df87cffcd993eaf85
at block: 781745 (Wed, 27 Jan 2021 17:50:33 UTC)
 datadir: /qdata/ethereum
 modules: admin:1.0 debug:1.0 eth:1.0 istanbul:1.0 miner:1.0 net:1.0 personal:1.0 rpc:1.0 txpool:1.0 web3:1.0

> eth.getBlock(30961)
{
  difficulty: 1,
  extraData: "0xd883010812846765746888676f312e31302e31856c696e757800000000000000f9010cf83f9408ff0cf0c2b8c7165620002f04b59606cf746f2894ac2b1e5275594ff84680b94871acc22acd1b87f694e38cdc5113361c0a5baa106df87cffcd993eaf85b841c14863b697eaa886aeee97dfbcd8d629f4a59c412c6b03dbe8a1558aab7b19f037ac533c446715241599af72ba2637cc39f9939c3c11df8bb9774b5a09d84bbf00f886b8413baa07fbfeb6d94ab2588ef009b01f191d815ead762617d28bf33df03aa8a309296af8de3fac7f787551dffb101a422b52fedcc5f98dbef32c3ed41bcfeed1cc01b8410000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000",
  gasLimit: 804247552,
  gasUsed: 0,
  hash: "0x4e0899c24f209ad9f7aca966bb6340c6e6f065b3085a74dcf17b5fe8a02877e9",
  logsBloom: "0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000",
  miner: "0x0000000000000000000000000000000000000000",
  mixHash: "0x63746963616c2062797a616e74696e65206661756c7420746f6c6572616e6365",
  nonce: "0x0000000000000000",
  number: 30961,
  parentHash: "0xc322d87a40c9648c3385e18f70439e335d3b289d340bd5f1aadf01a7da441427",
  receiptsRoot: "0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421",
  sha3Uncles: "0x1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347",
  size: 816,
  stateRoot: "0x7420bf7b4f32948b274529cc45080091e1392c5f1e28e4a54ed3e626ef53b20e",
  timestamp: 1585875696,
  totalDifficulty: 30962,
  transactions: [],
  transactionsRoot: "0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421",
  uncles: []
}
  • Error during import:
INFO [01-27|17:14:41.532] Imported new chain segment               blocks=2500 txs=0 mgas=0.000 elapsed=5.819s mgasps=0.000 number=30000 hash=afdda1…8430c0 age=9mo4w1d    cache=1.28kB
ERROR[01-27|17:14:43.941] not a valid address                      err="recovery failed"
ERROR[01-27|17:14:43.942]
########## BAD BLOCK #########
Chain config: {ChainID: 1700625682 Homestead: 0 DAO: <nil> DAOSupport: false EIP150: 0 EIP155: 0 EIP158: 0 Byzantium: 0 IsQuorum: true Constantinople: 0 TransactionSizeLimit: 128 MaxCodeSize: 0 Engine: istanbul}

Number: 30961
Hash: 0x4e0899c24f209ad9f7aca966bb6340c6e6f065b3085a74dcf17b5fe8a02877e9


Error: invalid signature
##############################

ERROR[01-27|17:14:43.943] Import error                             err="invalid block 32500: invalid signature"

Full Import Log

  • Decoded ExtraData is:
# ./istanbul extra decode --extradata "0xd883010812846765746888676f312e31302e31856c696e757800000000000000f9010cf83f9408ff0cf0c2b8c7165620002f04b59606cf746f2894ac2b1e5275594ff84680b94871acc22acd1b87f694e38cdc5113361c0a5baa106df87cffcd993eaf85b841c14863b697eaa886aeee97dfbcd8d629f4a59c412c6b03dbe8a1558aab7b19f037ac533c446715241599af72ba2637cc39f9939c3c11df8bb9774b5a09d84bbf00f886b8413baa07fbfeb6d94ab2588ef009b01f191d815ead762617d28bf33df03aa8a309296af8de3fac7f787551dffb101a422b52fedcc5f98dbef32c3ed41bcfeed1cc01b8410000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000"
vanity:  0xd883010812846765746888676f312e31302e31856c696e757800000000000000
validator:  0x08FF0Cf0C2B8c7165620002f04b59606Cf746f28
validator:  0xAc2b1E5275594ff84680B94871aCc22acd1B87F6
validator:  0xe38Cdc5113361C0A5baa106dF87cFfCD993EAF85
seal: 0xc14863b697eaa886aeee97dfbcd8d629f4a59c412c6b03dbe8a1558aab7b19f037ac533c446715241599af72ba2637cc39f9939c3c11df8bb9774b5a09d84bbf00
committed seal:  0x3baa07fbfeb6d94ab2588ef009b01f191d815ead762617d28bf33df03aa8a309296af8de3fac7f787551dffb101a422b52fedcc5f98dbef32c3ed41bcfeed1cc01
committed seal:  0x0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000

vdamle and others added 7 commits January 27, 2021 23:11
When a commit message is processed, if the Proposal is missing due to
some reason, we should not generate a Zero address Committed Seal. Such
an incorrect seal causes `BAD BLOCK` failure with an `invalid signature`
error during operations such as Block import
@vdamle
Copy link
Contributor Author

vdamle commented Sep 29, 2021

Hi team - Is there any input on this issue and proposed fix? We have seen this in another customer environment recently, running quorum-v21.4.2:

DEBUG[09-26|00:04:41.108] Queued delivered header or block         peer=istanbul         number=10472896 hash="91c2b5…64a051" queued=1
DEBUG[09-26|00:04:41.115] Importing propagated block               peer=istanbul         number=10472896 hash="91c2b5…64a051"
ERROR[09-26|00:04:41.122] not a valid address                      err="recovery failed"
DEBUG[09-26|00:04:41.122] Propagated block verification failed     peer=istanbul         number=10472896 hash="91c2b5…64a051" err="invalid signature"

@vdamle
Copy link
Contributor Author

vdamle commented Sep 30, 2021

Thanks for the review @ricardolyn . I've added a test that attempts to finalize a Commit message without a valid proposal.

@ricardolyn ricardolyn self-assigned this Sep 30, 2021
@ricardolyn
Copy link
Contributor

@vdamle can you please update with the latest master?

ricardolyn
ricardolyn previously approved these changes Sep 30, 2021
Co-authored-by: baptiste-b-pegasys <85155432+baptiste-b-pegasys@users.noreply.github.com>
@ricardolyn ricardolyn enabled auto-merge (squash) September 30, 2021 13:28
@ricardolyn ricardolyn merged commit 743e4a1 into Consensys:master Sep 30, 2021
vdamle added a commit to kaleido-io/quorum that referenced this pull request Oct 5, 2022
Consensys#1118 fixed an issue due to
which some blocks could be produced with an invalid seal in the list
of signers. This change ignores the error during block import/sync.
@lastperson
Copy link

Hey @vdamle could you advise how one should sync now with a chain that already has blocks with a zero committed seal (made before this fix)?

@lastperson
Copy link

lastperson commented Oct 13, 2022

Ambisafe@36aa739 here is the fix we used.

@vdamle
Copy link
Contributor Author

vdamle commented Oct 13, 2022

@lastperson My bad, I tagged only one commit with this issue, there are 2 commits on the branch you looked at, this view might help: https://github.com/kaleido-io/quorum/compare/6c76d8165eda4560e1f87ff0efd9a9a533a9d206..disable-signature-check

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants