Refactor/proto wallet deprecation#164
Open
ALiberalVoluntarist wants to merge 5 commits into
Open
Conversation
…nstruct per-output BEEF _parse_binary_response used Reader.read_var_int() (returns bytes) instead of read_var_int_num() (returns int), causing TypeError on any binary output-list response. Also reconstructs individual BEEF from the trailing combined BEEF instead of discarding it. Based on PR #162 by external contributor, with ruff formatting applied. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…md, and PR template Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The JSON response path in HTTPSOverlayLookupFacilitator.lookup() was calling response.json() but discarding the result, always returning an empty LookupAnswer. This meant any overlay host returning JSON (rather than binary) would produce no outputs. Added _parse_json_response() and _coerce_bytes() to properly handle the output-list JSON format. The beef field encoding differs across server implementations: - TS overlay-express: number[] (JS byte array, e.g. [222, 173, 190, 239]) - Go overlay-services: base64 string (Go []byte JSON default, e.g. "3q2+7w==") - hex string is also accepted as a fallback Note: the binary path (application/octet-stream) is triggered by the X-Aggregation: yes header, which this SDK always sends. The Go overlay server does not implement the binary path and always returns JSON. The TS overlay server supports both, selecting based on X-Aggregation. Tests: 10 cases covering number[], base64, hex, context, freeform, and edge cases. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…recate non-crypto methods Align ProtoWallet with TS/Go SDK architecture where it only provides cryptographic operations (9 methods). The 21 wallet-management methods (create_action, sign_action, list_outputs, etc.) now emit DeprecationWarning. Also replace ProtoWallet fallbacks in identity modules with ValueError, and fix verifiable_certificate.py to import the real WalletInterface Protocol. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of Changes
Align py-sdk
ProtoWalletwith TS/Go SDK architecture where ProtoWallet is a standalone cryptographic-operations-only class (9 methods), not a fullWalletInterfaceimplementation.Changes:
WalletInterfaceinheritance fromProtoWallet(class ProtoWallet(WalletInterface):→class ProtoWallet:)DeprecationWarningto 21 wallet-management methods (e.g.create_action,sign_action,list_outputs,internalize_action, etc.) that don't belong on ProtoWallet per TS/Go SDK designProtoWalletauto-creation fallbacks inIdentityClientandContactsManagerwithValueError("wallet is required")WalletInterfaceclass inverifiable_certificate.pywith the real Protocol import frombsv.wallet.wallet_interfaceKept (9 crypto-core methods, no deprecation):
get_public_key,encrypt,decrypt,create_signature,verify_signature,create_hmac,verify_hmac,reveal_counterparty_key_linkage,reveal_specific_key_linkageLinked Issues / Tickets
Related to TS/Go SDK architecture alignment. See also: #159 (
internalize_actionBEEF parse bug caused by ProtoWallet implementing methods it shouldn't own).Testing Procedure
All 21 deprecated methods emit
DeprecationWarningwithstacklevel=2IdentityClient(wallet=None)andContactsManager(wallet=None)now raiseValueErrorverifiable_certificate.pyimports the realWalletInterfaceProtocolI have added new unit tests
All tests pass locally (5329 passed, 262 skipped, 0 failed)
I have tested manually in my local environment
Checklist:
CHANGELOG.mdwith my changes