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 prover commitment serialization #1031

Merged
merged 2 commits into from
Aug 21, 2024

Conversation

yaziciahmet
Copy link
Contributor

Description

This pr fixes the DaData ser/de. BitcoinService serialized the inner values of DaData, while other components tried to deserialize it using DaData::try_from_slice.

@yaziciahmet yaziciahmet added the HOLD-MERGE PR is not draft but should not be merged yet label Aug 21, 2024
@yaziciahmet yaziciahmet self-assigned this Aug 21, 2024
Copy link

codecov bot commented Aug 21, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.9%. Comparing base (1bacc1e) to head (024f878).
Report is 2 commits behind head on nightly.

Additional details and impacted files
Files Coverage Δ
crates/bitcoin-da/src/service.rs 43.6% <ø> (ø)
crates/prover/src/prover_service/parallel/mod.rs 91.8% <100.0%> (ø)

@eyusufatik
Copy link
Member

I'd expect the bitcoin da verifier tests to fail?

Do you know if bitcoin da parsers should also change?

@yaziciahmet yaziciahmet force-pushed the yaziciahmet/fix-prover-commitment-deser branch from 0485f25 to 9744817 Compare August 21, 2024 13:46
@yaziciahmet yaziciahmet force-pushed the yaziciahmet/fix-prover-commitment-deser branch from 75d4229 to 024f878 Compare August 21, 2024 17:24
@yaziciahmet
Copy link
Contributor Author

@eyusufatik This is specific to method send_transaction_with_fee_rate, which is not used anywhere other than the DA queue worker, so there are no tests for this.

Copy link
Contributor

@kpp kpp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay as a tmp fix to make things work. However a proper fix needs to be implemented. Will do tomorrow.

@yaziciahmet yaziciahmet merged commit 289cff7 into nightly Aug 21, 2024
12 checks passed
@yaziciahmet yaziciahmet deleted the yaziciahmet/fix-prover-commitment-deser branch August 21, 2024 18:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
HOLD-MERGE PR is not draft but should not be merged yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants