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

feat: Validate coinbase outputs #158

Merged
merged 16 commits into from
Sep 14, 2024

Conversation

manlikeHB
Copy link
Contributor

Copy link

vercel bot commented Sep 8, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
raito ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 14, 2024 10:25am

src/validation/coinbase.cairo Outdated Show resolved Hide resolved
src/validation/coinbase.cairo Outdated Show resolved Hide resolved
src/validation/coinbase.cairo Outdated Show resolved Hide resolved
src/validation/coinbase.cairo Outdated Show resolved Hide resolved
src/validation/coinbase.cairo Outdated Show resolved Hide resolved
src/validation/coinbase.cairo Outdated Show resolved Hide resolved
src/validation/coinbase.cairo Outdated Show resolved Hide resolved
@m-kus
Copy link
Collaborator

m-kus commented Sep 9, 2024

@manlikeHB the task also includes calculating the wtxid commitment.
There is already wtxid merkle root calculated (see compute_and_validate_tx_data), the algorithm for calculating the commitment described in https://learnmeabitcoin.com/technical/transaction/wtxid/#commitment

Add unit tests to ensure that txid commitment is calculated correctly

src/validation/coinbase.cairo Outdated Show resolved Hide resolved
@manlikeHB
Copy link
Contributor Author

manlikeHB commented Sep 10, 2024

@m-kus, Is there a way I can get minimal/test blocks with segwit transactions to test the correctness of the wtxid commitment being calculated?

@m-kus
Copy link
Collaborator

m-kus commented Sep 10, 2024

@m-kus, Is there a way I can get minimal/test blocks with segwit transactions to test the correctness of the wtxid commitment being calculated?

There is now full_757738 test which applies a block with segwit transactions in main, you just need to rebase.
You can also take some data from it for a unit test.

@maciejka
Copy link
Collaborator

@m-kus @maciejka, I feel the issue I am facing is not from the validate_coinbase function because the unit test passes in test_validate_coinbase_with_segwit_tx test using mock coinbase transaction data and a pre-calculated wtxid root hash.

I am trying to debug but that's a bit hard since I can't log/print values to the terminal while running the integration test.

Hi, it is difficult to debug indeed. I pushed a fix to print test output. You should be able to debug it now. Don't hesitate to ask for help if you feel blocked next time!

- refac: represent witness reserved value in string literal
@m-kus m-kus merged commit 89c8055 into keep-starknet-strange:main Sep 14, 2024
6 checks passed
@manlikeHB manlikeHB deleted the validate-coinbase-outputs branch September 14, 2024 10:33
This pull request was closed.
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.

3 participants