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

Add error tests #524

Merged
merged 1 commit into from
May 2, 2024
Merged

Add error tests #524

merged 1 commit into from
May 2, 2024

Conversation

reez
Copy link
Collaborator

@reez reez commented May 1, 2024

Description

Chores+Tests:

  • Alphabetize Errors in bdk.udl
  • Alphabetize Errors in error.rs
  • Alphabetize Tests in error.rs
  • Add missing Tests in error.rs
  • Format Tests in error.rs to use vector based approach
  • Format naming of different types of things like:
    • fn from(error:
    • ParseError::WitnessVersion(e)
    • WitnessVersion { error_message: String })
      ... to all have consistency.

Notes to the reviewers

Going to stop here because diff is becoming too large "Large diffs are not rendered by default." for error.rs, and don't want to make this any harder than it already might be to review because of all the formatting changes.

Changelog notice

Checklists

All Submissions:

  • I've signed all my commits
  • I followed the contribution guidelines
  • I ran cargo fmt and cargo clippy before committing

New Features:

  • I've added tests for the new feature
  • I've added docs for the new feature

Bugfixes:

  • This pull request breaks the existing API
  • I've added tests to reproduce the issue which are now passing
  • I'm linking the issue being fixed by this PR

Summary by CodeRabbit

  • New Features
    • Enhanced error reporting across various interfaces, improving clarity and detail in error messages for users.
  • Bug Fixes
    • Removed outdated error methods and added new ones to better align with current functionalities and user needs.
  • Refactor
    • Updated error handling methods to provide more specific feedback, aiding in quicker issue resolution for users.

@reez reez linked an issue May 1, 2024 that may be closed by this pull request
8 tasks
@reez reez self-assigned this May 1, 2024
@reez reez marked this pull request as ready for review May 1, 2024 21:05
Copy link

coderabbitai bot commented May 1, 2024

Walkthrough

The recent update primarily overhauls error handling across various interfaces in the bdk-ffi module. Changes include the addition, removal, and modification of error types and methods across multiple interfaces, enhancing clarity and functionality in error reporting. This reflects a broader effort to improve the robustness and user feedback mechanisms within the system.

Changes

File Path Change Summary
bdk-ffi/src/bdk.udl Extensive updates to error interfaces, adding and refining methods and types for improved error handling and reporting. Includes changes in AddressError, Bip32Error, CreateTxError, DescriptorError, ExtractTxError, FeeRateError, PersistenceError, PsbtParseError, SignerError, TransactionError, TxidParseError.

Recent Review Details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits Files that changed from the base of the PR and between 6522dfd and 75d155c.
Files selected for processing (2)
  • bdk-ffi/src/bdk.udl (5 hunks)
  • bdk-ffi/src/error.rs (15 hunks)
Files not summarized due to errors (1)
  • bdk-ffi/src/error.rs: Error: Message exceeds token limit
Additional comments not posted (24)
bdk-ffi/src/bdk.udl (13)

8-17: Added multiple new methods to AddressError to cover a broader range of address-related errors. Ensure that these new methods are integrated into the error handling logic where addresses are processed or validated.


23-32: Updated error message parameters in Bip32Error for methods Secp256k1, Base58, and Hex. This should provide more detailed feedback on errors. Verify that the updated parameters are correctly used in the error handling logic.


36-41: Introduced new methods in Bip39Error to handle specific mnemonic-related errors. This enhancement should improve the clarity and specificity of error messages related to mnemonic processing.


51-52: Added a new method Include to CannotConnectError. This method seems to be intended for errors related to blockchain reorganization or similar scenarios. Confirm that this method is used appropriately in the context of blockchain connection errors.


Line range hint 57-78: Updated and added several methods in CreateTxError to handle transaction creation errors more effectively. These changes should enhance the robustness of transaction error handling. Ensure that these methods are used in the transaction creation logic.


82-95: Significant enhancements to DescriptorError with the addition of multiple new methods. These methods are crucial for handling errors related to transaction descriptors. Confirm that these methods are integrated into the descriptor validation and processing logic.


121-125: Added new methods to ExtractTxError to handle specific transaction extraction errors. These additions should improve the granularity of error reporting during transaction extraction processes.


129-130: Introduced a new enumeration FeeRateError with a type ArithmeticOverflow. This is a critical addition for handling errors related to fee rate calculations. Ensure that this error type is used in fee rate computation logic.


134-135: Added a new method Write to PersistenceError. This method is likely used for errors related to writing data to storage. Confirm that this method is appropriately used in the context of data persistence operations.


140-141: Updated PsbtParseError with new methods PsbtEncoding and Base64Encoding to handle specific parsing errors. These updates should enhance the clarity and specificity of error messages during PSBT parsing.


157-159: Updated error message parameters in SignerError for methods SighashError, MiniscriptPsbt, and External. This should provide more detailed feedback on signing errors. Verify that the updated parameters are correctly used in the error handling logic.

Verification successful

This script will first confirm the definitions of the SignerError variants in the bdk.udl file to ensure they are present as expected. Then, it will search for their usage in the Rust files, providing some context lines to help understand how they are used.


The verification process has confirmed that the updated error message parameters in the SignerError variants (SighashError, MiniscriptPsbt, and External) are indeed being used correctly within the error handling logic in the codebase. The script outputs show that these variants are being utilized to handle different types of errors, with the error messages being appropriately formatted and passed.

Here are the specific findings:

  • SighashError: The error message is converted to a string and passed to the SignerError::SighashError variant.
  • MiniscriptPsbt: The error message is formatted and passed to the SignerError::MiniscriptPsbt variant.
  • External: The error message is directly passed as a string to the SignerError::External variant.

These implementations align with the intended use of the updated parameters, ensuring that detailed feedback on signing errors is provided.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify that the updated error message parameters in `SignerError` are used correctly in the error handling logic.

# Find files that use the `SignerError` and check for correct usage of the updated parameters.
rg --files-with-matches --type rust $'SignerError' | while read file; do
  # Test: Search for the usage of updated methods with parameters. Expect: Correct usage.
  ast-grep --lang rust --pattern $'SignerError::SighashError($_)' "$file"
  ast-grep --lang rust --pattern $'SignerError::MiniscriptPsbt($_)' "$file"
  ast-grep --lang rust --pattern $'SignerError::External($_)' "$file"
done

Length of output: 862



Script:

#!/bin/bash
# Description: Re-verify the usage of updated error message parameters in `SignerError` methods.

# First, ensure that the methods are defined in the codebase.
echo "Checking definitions of SignerError variants:"
ast-grep --lang rust --pattern $'enum SignerError { $$$ SighashError($_), $$$ }' bdk-ffi/src/bdk.udl
ast-grep --lang rust --pattern $'enum SignerError { $$$ MiniscriptPsbt($_), $$$ }' bdk-ffi/src/bdk.udl
ast-grep --lang rust --pattern $'enum SignerError { $$$ External($_), $$$ }' bdk-ffi/src/bdk.udl

# If defined, check for their usage in the codebase.
echo "Searching for usage in the codebase:"
rg --type rust 'SignerError::SighashError' -A 3 -B 3
rg --type rust 'SignerError::MiniscriptPsbt' -A 3 -B 3
rg --type rust 'SignerError::External' -A 3 -B 3

Length of output: 3410


163-170: Significant updates to TransactionError with the addition of several new methods. These methods are crucial for handling a variety of transaction-related errors. Confirm that these methods are integrated into the transaction processing logic.


174-175: Added a new method InvalidTxid to TxidParseError. This method is essential for handling errors related to invalid transaction identifiers. Ensure that this method is used in the transaction identification logic.

bdk-ffi/src/error.rs (11)

31-63: The addition of detailed error types and messages for AddressError enhances clarity and debuggability. Good use of structured errors with contextual information.


Line range hint 70-99: The updates to Bip32Error, including the addition of detailed error messages, improve the error information provided to the user. Ensure that all potential error cases from the underlying libraries are handled.


101-117: The new error types added to Bip39Error cover a wider range of issues that can occur with BIP39 operations, which is a positive improvement for error handling in cryptographic operations.


Line range hint 136-200: The modifications and additions to CreateTxError enhance the granularity of error reporting in transaction creation processes. It's crucial to ensure that these changes are reflected in the transaction handling logic to utilize the new error types effectively.


204-239: The expansion of DescriptorError with more specific error types like InvalidHdKeyPath and MultiPath is beneficial. These changes should help in pinpointing issues in descriptor processing more accurately.


294-308: The addition of specific error cases such as AbsurdFeeRate and SendingTooMuch in ExtractTxError is commendable as it addresses precise failure scenarios in transaction extraction, enhancing the robustness of error handling.


317-319: Adding a Write error type to PersistenceError with a detailed error message parameter will help in identifying issues related to data persistence more clearly.


323-328: The new error types PsbtEncoding and Base64Encoding in PsbtParseError provide more detailed insights into where the parsing process may fail, which is crucial for debugging PSBT-related issues.


332-373: The updates to SignerError, including the addition of errors like MissingWitnessScript and NonStandardSighash, enhance the error handling capabilities of the signing process. These changes are essential for a more robust and error-resilient signing mechanism.


402-404: The introduction of InvalidTxid in TxidParseError is a specific and useful addition for handling errors related to transaction ID parsing.


408-434: The changes in WalletCreationError, including more specific errors like InvalidMagicBytes and LoadedNetworkDoesNotMatch, provide better error feedback during wallet creation and loading processes. This is crucial for troubleshooting and ensuring data integrity.


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

Note: Auto-reply has been disabled for this repository by the repository owner. The CodeRabbit bot will not respond to your replies unless it is explicitly tagged.

  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@reez reez requested a review from thunderbiscuit May 1, 2024 21:05
@reez reez mentioned this pull request May 2, 2024
8 tasks
Copy link
Member

@thunderbiscuit thunderbiscuit left a comment

Choose a reason for hiding this comment

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

ACK 75d155c. Love the work on renaming the fields all the same.

On those big "moving-around" PRs it's hard to tell if it all is still there because of the way the diff is displayed, but I think it's all there. I think we'd get compilation errors if it wasn't anyway.

Great cleanup, thanks a bunch!

@thunderbiscuit thunderbiscuit merged commit 75d155c into bitcoindevkit:master May 2, 2024
25 checks passed
@reez reez deleted the issue-495 branch May 2, 2024 18:08
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.

Remove Alpha3Error
2 participants