Remove pre-1.0 code #523
Remove pre-1.0 code #523
Conversation
WalkthroughThe recent updates to the Changes
Recent Review DetailsConfiguration used: CodeRabbit UI Files selected for processing (4)
Files skipped from review due to trivial changes (2)
Additional comments not posted (2)
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? TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Out of diff range and nitpick comments (3)
bdk-ffi/src/bitcoin.rs (2)
[!TIP]
Codebase VerificationThe verification process has revealed that the removed method
Address::from_scriptis still referenced in the codebase within a commented section inbdk-ffi/src/wallet.rs. Although this does not impact the functionality, it could potentially lead to confusion. It is advisable to either update or remove this commented code to maintain clarity.
- File and location:
bdk-ffi/src/wallet.rs(within commented code)Analysis chain
Line range hint
38-38: Confirm removal ofAddress::from_script()andAddress::payload().Ensure that no residual references to these methods exist elsewhere in the codebase.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Ensure no residual references to removed methods exist in the codebase. # Search for 'from_script' and 'payload' in the entire codebase. Expect: No matches. rg --files-with-matches --type rust 'from_script|payload'Length of output: 81
Script:
#!/bin/bash # Description: Extract and display the lines from bdk-ffi/src/wallet.rs where 'from_script' and 'payload' are mentioned. # Use ripgrep to find the lines containing 'from_script' or 'payload' and display 2 lines of context around each match. rg --context 2 'from_script|payload' bdk-ffi/src/wallet.rsLength of output: 300
[!TIP]
Codebase VerificationThe output from the script reveals that the methods
txid,combine,fee_amount,fee_rate, andjson_serializeare still referenced in various files within thebdk-ffidirectory. These references appear in active code segments, not just in comments or deprecated sections. This indicates that the removal of these methods from thePsbtclass could potentially lead to issues in other parts of the codebase that rely on these methods.Given the findings, it is clear that there are residual references to the removed methods, which could affect the functionality of the codebase. The developers need to address these references either by updating them to align with the new method implementations or by revising the parts of the code that depend on these removed methods.
Conclusion:
- Several files in the codebase still use the old function signatures.
- The following files contain references to the removed methods and need attention:
bdk-ffi/src/error.rsbdk-ffi/src/keys.rsbdk-ffi/src/descriptor.rsbdk-ffi/src/types.rsbdk-ffi/src/wallet.rsbdk-ffi/src/bitcoin.rsAnalysis chain
Line range hint
156-156: Confirm removal ofPsbt::txid(),Psbt::combine(),Psbt::fee_amount(),Psbt::fee_rate(), andPsbt::json_serialize().Ensure that no residual references to these methods exist elsewhere in the codebase.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Ensure no residual references to removed methods exist in the codebase. # Search for 'txid', 'combine', 'fee_amount', 'fee_rate', 'json_serialize' in the entire codebase. Expect: No matches. rg --files-with-matches --type rust 'txid|combine|fee_amount|fee_rate|json_serialize'Length of output: 220
Script:
#!/bin/bash # Description: Inspect the contents of the files where removed methods are referenced to understand the context. # List occurrences of 'txid', 'combine', 'fee_amount', 'fee_rate', 'json_serialize' in the specified files. rg 'txid|combine|fee_amount|fee_rate|json_serialize' bdk-ffi/src/error.rs bdk-ffi/src/keys.rs bdk-ffi/src/descriptor.rs bdk-ffi/src/types.rs bdk-ffi/src/wallet.rs bdk-ffi/src/bitcoin.rsLength of output: 4477
bdk-ffi/src/wallet.rs (1)
[!TIP]
Codebase VerificationThe extensive output from the script reveals that the methods
new,is_mine,get_address,get_internal_address,get_balance,sign,list_transactions, andlist_unspentare still referenced across multiple files in the codebase. This includes active code usage, not just comments or documentation. This indicates that the removal of these methods from theWalletclass might have been premature or incomplete, as they are still actively used or required by other parts of the codebase.Here are the specific issues found:
- Active method implementations and calls: The methods are not only declared but also implemented and called in various parts of the codebase. For example,
Wallet::new,Wallet::get_address, andWallet::signare actively used inwallet.rs.- References in other files: Other files like
esplora.rs,bitcoin.rs,types.rs,keys.rs,descriptor.rs, anderror.rsalso reference these methods either in their implementations or in related functionality.Given these findings, it is clear that the removal of these methods has not been fully accounted for in the codebase. This could potentially lead to runtime errors if the code attempts to call these now-nonexistent methods.
Conclusion
Several files in the codebase still actively use the methods that were supposed to be removed. This could lead to significant functionality issues or crashes. It is recommended to either restore these methods in the
Walletclass or refactor the other parts of the codebase to remove the dependencies on these methods.
- Files and methods that need attention:
bdk-ffi/src/wallet.rs: References to all removed methodsbdk-ffi/src/esplora.rs: References tonewbdk-ffi/src/bitcoin.rs: References tonew,get_addressbdk-ffi/src/types.rs: References toget_addressbdk-ffi/src/keys.rs: References tonewbdk-ffi/src/descriptor.rs: References tonewbdk-ffi/src/error.rs: References tonew,signAnalysis chain
Line range hint
11-11: Confirm removal of commented-out methods inWalletclass.Ensure that no residual references to these methods exist elsewhere in the codebase.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Ensure no residual references to removed methods exist in the codebase. # Search for 'new', 'is_mine', 'get_address', 'get_internal_address', 'get_balance', 'sign', 'list_transactions', 'list_unspent' in the entire codebase. Expect: No matches. rg --files-with-matches --type rust 'new|is_mine|get_address|get_internal_address|get_balance|sign|list_transactions|list_unspent'Length of output: 288
Script:
#!/bin/bash # Description: Extract the lines of code from each file where the removed methods are referenced to understand the context. # List of methods removed methods='new|is_mine|get_address|get_internal_address|get_balance|sign|list_transactions|list_unspent' # Files to check files="bdk-ffi/src/wallet.rs bdk-ffi/src/esplora.rs bdk-ffi/src/bitcoin.rs bdk-ffi/src/types.rs bdk-ffi/src/keys.rs bdk-ffi/src/descriptor.rs bdk-ffi/src/error.rs" # Extract relevant lines from each file for file in $files; do echo "File: $file" rg --context 2 "$methods" $file echo "----------------------------------------" doneLength of output: 31489
Instead of closing an issue, this PR opens one: #522.
I just thought I'd be cleaner to keep track of what we want to bring back in an issue (or maybe multiple issues?) rather than just old commented out code that's getting very stale at this point. The basic idea is that every codeblock that is removed here gets an associated todo item in #522.
Let me know what you think.
Summary by CodeRabbit