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

Fix invalid pre-fetched header broadcast #8442

Merged
merged 3 commits into from
Oct 12, 2023
Merged

Conversation

mh0lt
Copy link
Contributor

@mh0lt mh0lt commented Oct 11, 2023

Fixes and issue with Polygon validators where locally mined blocks are broadcast with invalid header hashes because the NewBlock message constructor was removing the ReceiptHash which contributed to the header hash.

The results in the bor header validation code not being able to correctly identify the signer of the header - so header validation fails.

This also likely fixes part of the bogon-block issue which was identified by the polygon team.

@mh0lt mh0lt added the polygon label Oct 11, 2023
@mh0lt mh0lt merged commit 6f7186e into devel Oct 12, 2023
4 checks passed
@mh0lt mh0lt deleted the fix_invalid_header_broadcast branch October 12, 2023 07:27
yperbasis pushed a commit that referenced this pull request Oct 12, 2023
Fixes and issue with Polygon validators where locally mined blocks are
broadcast with invalid header hashes because the NewBlock message
constructor was removing the ReceiptHash which contributed to the header
hash.

The results in the bor header validation code not being able to
correctly identify the signer of the header - so header validation
fails.

This also likely fixes part of the bogon-block issue which was
identified by the polygon team.
yperbasis added a commit that referenced this pull request Oct 12, 2023
Fixes and issue with Polygon validators where locally mined blocks are
broadcast with invalid header hashes because the NewBlock message
constructor was removing the ReceiptHash which contributed to the header
hash.

The results in the bor header validation code not being able to
correctly identify the signer of the header - so header validation
fails.

This also likely fixes part of the bogon-block issue which was
identified by the polygon team.

Co-authored-by: Mark Holt <135143369+mh0lt@users.noreply.github.com>
yperbasis pushed a commit that referenced this pull request Oct 13, 2023
… with empty receipt hash (#8467)

## Description

This PR is a follow-up of #8442.

1. The previous PR resolved the issue where newly mined blocks had their
receipt hash set to an empty root hash. But I think it introduced an
issue where the new blocks include no transactions. So this PR addresses
that.

2. On top of that, this new PR introduces a validation step for the
receipt hash when peers receive new blocks at the p2p layer. If the
receipt hash is set to the empty root hash, but the block contains
transactions, it will now raise an error.

## Test

- When running a local erigon devnet with the new receipt hash check and
before #8442 fix (with a few more logs).

**=> error message: `newBlock66: block has empty receipt hash:
56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421 but it
includes 2 transactions`**

```sh
Received new block request: &{Block:0xc000bd0230 TD:+7891}
Block header: &{ParentHash:0x998f5be682d09b9d57dbf9c42032c23754bc9569d39670a1d7479f74946df0a9 UncleHash:0x1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347 Coinbase:0x0000000000000000000000000000000000000000 Root:0x422bd23663d6d52b94cd537491eaa2b250b03e5e252b9c1dfabee5dc12673628 TxHash:0xe4ec65ce3aba6347c9e7139571cd23ec5e42b3c1952706a3d965a322699b9b5e ReceiptHash:0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421 Bloom:[0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0] Difficulty:+2 Number:+3945 GasLimit:30000000 GasUsed:70178 Time:1697192495 Extra:[0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 197 254 60 127 243 72 143 204 98 97 12 136 59 50 115 115 204 239 143 157 234 71 55 113 77 66 2 227 74 65 226 190 44 226 215 32 177 90 161 172 41 69 31 229 166 221 185 14 128 47 169 132 130 249 179 129 153 159 134 124 162 234 81 187 1] MixDigest:0x0000000000000000000000000000000000000000000000000000000000000000 Nonce:[0 0 0 0 0 0 0 0] AuRaStep:0 AuRaSeal:[] BaseFee:+8 WithdrawalsHash:<nil> BlobGasUsed:<nil> ExcessBlobGas:<nil> ParentBeaconBlockRoot:<nil> Verkle:false VerkleProof:[] VerkleKeyVals:[]}
Block body: &{Transactions:[0xc000cde100 0xc000cde200] Uncles:[] Withdrawals:[]}
DBUG[10-13|12:21:35.789] Handling incoming message                stream=RecvMessage err="newBlock66: block has empty receipt hash: 56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421 but it includes 2 transactions"
...
```

- When running a local erigon devnet with the new receipt hash check
after #8442 fix (with a few more logs).

**=> error message: `newBlock66: block has invalid transaction hash
...`**

```sh
Received new block request: &{Block:0xc002c38930 TD:+11963}
Block header: &{ParentHash:0x89a5134df82c85b38f2e02524143b575c44c5c6df444df9fc4d86c5972601ce0 UncleHash:0x1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347 Coinbase:0x0000000000000000000000000000000000000000 Root:0x1ab62696e1b585c8e084e12781e2f36f2591f91fe45e5b125df2468abd1f54e5 TxHash:0x6e7642d32215ab23f9096ed36463730ffa6c17c26dbba50c6100d64b5dee6349 ReceiptHash:0x9c8f5e4d71a8bb3ace2c0ce45e00357278fcac6da41b9e353872f106298cf6ec Bloom:[0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 8 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 8 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 2 0 0 0 16 0 0 0 0 0 32 0 0 0 0 0 0 0 16 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 128 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 8 0 2 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 2 0 0 0 0 0 0 0 0 0 0 16 0 16 0 0 0] Difficulty:+2 Number:+5981 GasLimit:30000000 GasUsed:70178 Time:1697193936 Extra:[0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 187 75 33 182 137 174 13 214 132 232 242 141 222 107 53 186 250 113 185 149 118 134 112 84 167 16 126 38 158 252 205 66 117 96 205 198 79 158 82 42 199 29 37 92 23 3 140 113 216 66 105 174 144 209 110 121 119 43 165 204 81 54 113 1] MixDigest:0x0000000000000000000000000000000000000000000000000000000000000000 Nonce:[0 0 0 0 0 0 0 0] AuRaStep:0 AuRaSeal:[] BaseFee:+8 WithdrawalsHash:<nil> BlobGasUsed:<nil> ExcessBlobGas:<nil> ParentBeaconBlockRoot:<nil> Verkle:false VerkleProof:[] VerkleKeyVals:[]}
Block body: &{Transactions:[] Uncles:[] Withdrawals:[]}
DBUG[10-13|12:45:36.908] Handling incoming message                stream=RecvMessage err="newBlock66: block has invalid transaction hash: have 56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421, exp: 6e7642d32215ab23f9096ed36463730ffa6c17c26dbba50c6100d64b5dee6349"
...
```

- When running a local erigon devnet with the new receipt hash check and
after this PR's fix (include transactions in block)

**=> no error message 🎉**

```sh
Received new block request: &{Block:0xc000d5a000 TD:+1227}
Block header: &{ParentHash:0xf5d6a19c8d56f230eeb8d2654e5562b934236136d5fcc50cbcde5c9ccc659810 UncleHash:0x1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347 Coinbase:0x0000000000000000000000000000000000000000 Root:0x9c17098ebfda6e59ceef147778367c628a44a6f1421ef21e844a9d8450f64a74 TxHash:0x507daf8bc5b329226614a0cf535273242cbf15d5ad582a4d5db0d5881732f791 ReceiptHash:0xaea5f86a33cba20385d4ce630b5f7ff613139b374718d89eafe224bd4b57e903 Bloom:[0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 2 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 2 0 0 0 0 0 0 4 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0] Difficulty:+2 Number:+613 GasLimit:20918370 GasUsed:54544 Time:1697195980 Extra:[0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 118 30 19 173 212 211 186 172 0 200 117 103 101 19 76 147 18 161 141 70 70 237 243 162 239 236 162 35 154 248 10 28 90 20 214 32 140 72 23 151 36 212 210 99 122 154 244 229 171 249 29 105 109 23 80 112 142 65 235 189 59 238 8 27 1] MixDigest:0x0000000000000000000000000000000000000000000000000000000000000000 Nonce:[0 0 0 0 0 0 0 0] AuRaStep:0 AuRaSeal:[] BaseFee:+8 WithdrawalsHash:<nil> BlobGasUsed:<nil> ExcessBlobGas:<nil> ParentBeaconBlockRoot:<nil> Verkle:false VerkleProof:[] VerkleKeyVals:[]}
Block body: &{Transactions:[0xc0000dc400 0xc0000dc600] Uncles:[] Withdrawals:[]}
...
```
yperbasis pushed a commit that referenced this pull request Oct 13, 2023
… with empty receipt hash (#8467)

## Description

This PR is a follow-up of #8442.

1. The previous PR resolved the issue where newly mined blocks had their
receipt hash set to an empty root hash. But I think it introduced an
issue where the new blocks include no transactions. So this PR addresses
that.

2. On top of that, this new PR introduces a validation step for the
receipt hash when peers receive new blocks at the p2p layer. If the
receipt hash is set to the empty root hash, but the block contains
transactions, it will now raise an error.

## Test

- When running a local erigon devnet with the new receipt hash check and
before #8442 fix (with a few more logs).

**=> error message: `newBlock66: block has empty receipt hash:
56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421 but it
includes 2 transactions`**

```sh
Received new block request: &{Block:0xc000bd0230 TD:+7891}
Block header: &{ParentHash:0x998f5be682d09b9d57dbf9c42032c23754bc9569d39670a1d7479f74946df0a9 UncleHash:0x1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347 Coinbase:0x0000000000000000000000000000000000000000 Root:0x422bd23663d6d52b94cd537491eaa2b250b03e5e252b9c1dfabee5dc12673628 TxHash:0xe4ec65ce3aba6347c9e7139571cd23ec5e42b3c1952706a3d965a322699b9b5e ReceiptHash:0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421 Bloom:[0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0] Difficulty:+2 Number:+3945 GasLimit:30000000 GasUsed:70178 Time:1697192495 Extra:[0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 197 254 60 127 243 72 143 204 98 97 12 136 59 50 115 115 204 239 143 157 234 71 55 113 77 66 2 227 74 65 226 190 44 226 215 32 177 90 161 172 41 69 31 229 166 221 185 14 128 47 169 132 130 249 179 129 153 159 134 124 162 234 81 187 1] MixDigest:0x0000000000000000000000000000000000000000000000000000000000000000 Nonce:[0 0 0 0 0 0 0 0] AuRaStep:0 AuRaSeal:[] BaseFee:+8 WithdrawalsHash:<nil> BlobGasUsed:<nil> ExcessBlobGas:<nil> ParentBeaconBlockRoot:<nil> Verkle:false VerkleProof:[] VerkleKeyVals:[]}
Block body: &{Transactions:[0xc000cde100 0xc000cde200] Uncles:[] Withdrawals:[]}
DBUG[10-13|12:21:35.789] Handling incoming message                stream=RecvMessage err="newBlock66: block has empty receipt hash: 56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421 but it includes 2 transactions"
...
```

- When running a local erigon devnet with the new receipt hash check
after #8442 fix (with a few more logs).

**=> error message: `newBlock66: block has invalid transaction hash
...`**

```sh
Received new block request: &{Block:0xc002c38930 TD:+11963}
Block header: &{ParentHash:0x89a5134df82c85b38f2e02524143b575c44c5c6df444df9fc4d86c5972601ce0 UncleHash:0x1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347 Coinbase:0x0000000000000000000000000000000000000000 Root:0x1ab62696e1b585c8e084e12781e2f36f2591f91fe45e5b125df2468abd1f54e5 TxHash:0x6e7642d32215ab23f9096ed36463730ffa6c17c26dbba50c6100d64b5dee6349 ReceiptHash:0x9c8f5e4d71a8bb3ace2c0ce45e00357278fcac6da41b9e353872f106298cf6ec Bloom:[0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 8 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 8 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 2 0 0 0 16 0 0 0 0 0 32 0 0 0 0 0 0 0 16 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 128 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 8 0 2 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 2 0 0 0 0 0 0 0 0 0 0 16 0 16 0 0 0] Difficulty:+2 Number:+5981 GasLimit:30000000 GasUsed:70178 Time:1697193936 Extra:[0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 187 75 33 182 137 174 13 214 132 232 242 141 222 107 53 186 250 113 185 149 118 134 112 84 167 16 126 38 158 252 205 66 117 96 205 198 79 158 82 42 199 29 37 92 23 3 140 113 216 66 105 174 144 209 110 121 119 43 165 204 81 54 113 1] MixDigest:0x0000000000000000000000000000000000000000000000000000000000000000 Nonce:[0 0 0 0 0 0 0 0] AuRaStep:0 AuRaSeal:[] BaseFee:+8 WithdrawalsHash:<nil> BlobGasUsed:<nil> ExcessBlobGas:<nil> ParentBeaconBlockRoot:<nil> Verkle:false VerkleProof:[] VerkleKeyVals:[]}
Block body: &{Transactions:[] Uncles:[] Withdrawals:[]}
DBUG[10-13|12:45:36.908] Handling incoming message                stream=RecvMessage err="newBlock66: block has invalid transaction hash: have 56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421, exp: 6e7642d32215ab23f9096ed36463730ffa6c17c26dbba50c6100d64b5dee6349"
...
```

- When running a local erigon devnet with the new receipt hash check and
after this PR's fix (include transactions in block)

**=> no error message 🎉**

```sh
Received new block request: &{Block:0xc000d5a000 TD:+1227}
Block header: &{ParentHash:0xf5d6a19c8d56f230eeb8d2654e5562b934236136d5fcc50cbcde5c9ccc659810 UncleHash:0x1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347 Coinbase:0x0000000000000000000000000000000000000000 Root:0x9c17098ebfda6e59ceef147778367c628a44a6f1421ef21e844a9d8450f64a74 TxHash:0x507daf8bc5b329226614a0cf535273242cbf15d5ad582a4d5db0d5881732f791 ReceiptHash:0xaea5f86a33cba20385d4ce630b5f7ff613139b374718d89eafe224bd4b57e903 Bloom:[0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 2 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 2 0 0 0 0 0 0 4 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0] Difficulty:+2 Number:+613 GasLimit:20918370 GasUsed:54544 Time:1697195980 Extra:[0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 118 30 19 173 212 211 186 172 0 200 117 103 101 19 76 147 18 161 141 70 70 237 243 162 239 236 162 35 154 248 10 28 90 20 214 32 140 72 23 151 36 212 210 99 122 154 244 229 171 249 29 105 109 23 80 112 142 65 235 189 59 238 8 27 1] MixDigest:0x0000000000000000000000000000000000000000000000000000000000000000 Nonce:[0 0 0 0 0 0 0 0] AuRaStep:0 AuRaSeal:[] BaseFee:+8 WithdrawalsHash:<nil> BlobGasUsed:<nil> ExcessBlobGas:<nil> ParentBeaconBlockRoot:<nil> Verkle:false VerkleProof:[] VerkleKeyVals:[]}
Block body: &{Transactions:[0xc0000dc400 0xc0000dc600] Uncles:[] Withdrawals:[]}
...
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant