-
Notifications
You must be signed in to change notification settings - Fork 28
Deposit script pushnum conversion #1224
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR refactors the deposit script parsing logic to properly handle Bitcoin push number opcodes (OP_PUSHNUM_1 through OP_PUSHNUM_16 and OP_PUSHNUM_NEG1) when extracting witness data from BitVM disprove scripts. The changes also improve logging practices by adding detailed debug logs, converting some warn-level logs to info-level, and protecting sensitive transaction data from production logs.
Key Changes:
- Introduced
collect_data_pushes_from_disprove_scriptfunction to properly convert numeric push opcodes to their byte representations - Added comprehensive test coverage for all Bitcoin push opcodes (PUSHBYTES, PUSHDATA1-4, and numeric opcodes)
- Improved logging hygiene by using appropriate log levels and protecting sensitive data with
#[cfg(test)]
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 9 comments.
| File | Description |
|---|---|
| core/src/verifier.rs | Added new function to handle push opcode conversion including numeric opcodes; refactored disprove script parsing; added debug logging for assertion data and disprove scripts; comprehensive test coverage for all push opcodes |
| core/src/tx_sender/mod.rs | Removed debug log that exposed raw transaction data |
| core/src/tx_sender/client.rs | Added test-only raw transaction logging with #[cfg(test)] guard for security |
| core/src/operator.rs | Changed log levels from warn to info for normal operational events; improved log message clarity |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
No description provided.