Add coinbase depth and tx id inclusion proof to BitcoinLightClient
#1863
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
plan is to merge #1935 then update genesises for tests that start with Fork2 in this PR. we might want to implement fork2 function here so that we can keep backwards compatibility easily. this would also mean we should update the bridge to use this new function. that can be this PR or another one. only after these we can merge this PR. then we'll need a followup PR for #1913 |
eyusufatik
requested changes
Feb 24, 2025
| @@ -32,6 +38,7 @@ contract BitcoinLightClient is IBitcoinLightClient { | |||
| blockNumber = _blockNumber; | |||
| } | |||
|
|
|||
| /// @notice Kept for backwards compatibility | |||
There was a problem hiding this comment.
add warning that this should not be called and calling it will invalidate L2 block after Fork2 activates
| /// @param _proof Merkle proof | ||
| /// @param _index Index of the transaction | ||
| /// @return If the transaction ID is included in the merkle root hash of the block | ||
| function verifyInclusionByTxId(uint256 _blockNumber, bytes32 _txId, bytes calldata _blockHeader, bytes calldata _proof, uint256 _index) external view returns (bool) { |
There was a problem hiding this comment.
sneaking in features I see :)
| }); | ||
| system_events | ||
| } | ||
|
|
||
| /// Populates system events based on the current soft confirmation info. | ||
| pub fn populate_system_events<'a>( |
| @@ -447,6 +450,7 @@ where | |||
|
|
|||
| let evm_txs = self.get_best_transactions()?; | |||
|
|
|||
| let coinbase_depth = short_header_proof.verify().unwrap().coinbase_txid_merkle_proof_height.into(); | |||
There was a problem hiding this comment.
shouldn't handle it like this
There was a problem hiding this comment.
what's wrong with it
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
BitcoinLightClientto solve the merkle tree leaf node weakness issue.verifyInclusionByTxIdfunction that acts the same asverifyInclusionbut usestxIdinstead ofwtxId, as such, it requires a Bitcoin header parameter that is later validated against stored Bitcoin block hash.Linked Issues
BitcoinLightClient::setBlockInfo()with coinbase txid merkle proof depth #1777Testing
Added unit tests for coinbase depth fail case and verifying inclusion through tx id.