From 18b3ff339082471937eec2ada2d319a9960a93d4 Mon Sep 17 00:00:00 2001 From: Kim De Mey Date: Mon, 18 Sep 2023 14:50:07 +0200 Subject: [PATCH] Correct the shortcut for ommershash verification post Shanghai (#1756) * Correct the shortcut for ommershash verification post Shanghai There was a faulty length check being done in the assumption that the uncles field was already the list. However it is still the RLP encoded data at that point, hence we check directly for the RLP encoded value for empty list. * Fix copy-paste error in withdrawal root verification And print out the hashes in txs root and withdrawal root verification --- fluffy/network/history/history_network.nim | 24 +++++++++++++--------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/fluffy/network/history/history_network.nim b/fluffy/network/history/history_network.nim index c5c19ea5d..d9584c12b 100644 --- a/fluffy/network/history/history_network.nim +++ b/fluffy/network/history/history_network.nim @@ -230,7 +230,8 @@ proc validateBlockBody( let calculatedTxsRoot = calcTxsRoot(body.transactions) if calculatedTxsRoot != header.txRoot: - return err("Invalid transactions root") + return err("Invalid transactions root: expected " & + $header.txRoot & " - got " & $calculatedTxsRoot) ok() @@ -239,23 +240,26 @@ proc validateBlockBody( Result[void, string] = ## Validate the block body against the txRoot, ommersHash and withdrawalsRoot ## from the header. - # Shortcutting the ommersHash calculation as uncles needs to be empty - # TODO: This is since post-merge, however, we would need an additional object - # type for that period to do this. - if body.uncles.len > 0: - return err("Invalid ommers hash") + # Shortcut the ommersHash calculation as uncles must be an RLP encoded + # empty list + if body.uncles.asSeq() != @[byte 0xc0]: + return err("Invalid ommers hash, uncles list is not empty") let calculatedTxsRoot = calcTxsRoot(body.transactions) if calculatedTxsRoot != header.txRoot: - return err("Invalid transactions root") + return err("Invalid transactions root: expected " & + $header.txRoot & " - got " & $calculatedTxsRoot) # TODO: This check is done higher up but perhaps this can become cleaner with # some refactor. doAssert(header.withdrawalsRoot.isSome()) - let calculatedWithdrawalsRoot = calcWithdrawalsRoot(body.withdrawals) - if calculatedWithdrawalsRoot != header.txRoot: - return err("Invalid transactions root") + let + calculatedWithdrawalsRoot = calcWithdrawalsRoot(body.withdrawals) + headerWithdrawalsRoot = header.withdrawalsRoot.get() + if calculatedWithdrawalsRoot != headerWithdrawalsRoot: + return err("Invalid withdrawals root: expected " & + $headerWithdrawalsRoot & " - got " & $calculatedWithdrawalsRoot) ok()