Skip to content

Conversation

@Quexington
Copy link
Contributor

@Quexington Quexington commented Dec 4, 2025

This PR removes the signing logic that was previously only accessible from the RPC into its own library. This minimizes the RPC logic and also provides this functionality to other wallet areas potentially.

(it also adds tests for the previously untested sign_message_by_id)


Note

Extracts message signing/verification into a shared util and refactors RPC and wallets to use it; adds tests for signing by NFT ID and address.

  • Wallet signing refactor:
    • Introduce chia/wallet/util/signing.py with sign_message() and verify_signature().
    • Remove ad-hoc sign_message methods from Wallet, DIDWallet, and NFTWallet; add helpers like convert_secret_key_to_synthetic() and current_p2_puzzle_hash().
  • RPC updates:
    • wallet_rpc_api.py: delegate verify_signature, sign_message_by_address, and sign_message_by_id to new signing util; simplify key retrieval and signing flow.
    • wallet_request_types.py: add signing_mode_enum helpers and properties; extend request/response types for signing.
  • Tests:
    • Add test_sign_message_by_nft_id (NFT signing across modes) and test_sign_message_by_address.
    • Update imports/usages accordingly.

Written by Cursor Bugbot for commit 3d85e99. This will update automatically on new commits. Configure here.

@Quexington Quexington requested a review from a team as a code owner December 4, 2025 21:42
@Quexington Quexington added the Changed Required label for PR that categorizes merge commit message as "Changed" for changelog label Dec 4, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Dec 4, 2025

File Coverage Missing Lines
chia/wallet/did_wallet/did_wallet.py 66.7% lines 906
chia/wallet/util/signing.py 93.5% lines 35, 37, 39
chia/wallet/wallet_request_types.py 91.3% lines 387-388
Total Missing Coverage
135 lines 6 lines 95%

@coveralls-official
Copy link

Pull Request Test Coverage Report for Build 19944879525

Details

  • 129 of 135 (95.56%) changed or added relevant lines in 8 files are covered.
  • 42 unchanged lines in 9 files lost coverage.
  • Overall coverage decreased (-0.02%) to 90.753%

Changes Missing Coverage Covered Lines Changed/Added Lines %
chia/wallet/did_wallet/did_wallet.py 2 3 66.67%
chia/wallet/wallet_request_types.py 21 23 91.3%
chia/wallet/util/signing.py 43 46 93.48%
Files with Coverage Reduction New Missed Lines %
chia/_tests/simulation/test_simulation.py 1 96.47%
chia/timelord/timelord_api.py 1 93.02%
chia/wallet/wallet_node.py 1 86.91%
chia/_tests/core/test_farmer_harvester_rpc.py 2 98.06%
chia/timelord/timelord_launcher.py 2 70.0%
chia/full_node/full_node.py 4 87.0%
chia/daemon/server.py 7 80.62%
chia/timelord/timelord.py 11 72.0%
chia/server/node_discovery.py 13 79.79%
Totals Coverage Status
Change from base Build 19938390251: -0.02%
Covered Lines: 102485
Relevant Lines: 112753

💛 - Coveralls

Copy link
Contributor

@prozacchiwawa prozacchiwawa left a comment

Choose a reason for hiding this comment

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

makes sense. that was a fair amount of duplication in the individual wallet code.

@emlowe
Copy link
Contributor

emlowe commented Dec 8, 2025

cursor review

@TheLastCicada TheLastCicada merged commit 47ed91c into main Dec 9, 2025
417 of 420 checks passed
@TheLastCicada TheLastCicada deleted the quex.extract_signing_library branch December 9, 2025 19:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Changed Required label for PR that categorizes merge commit message as "Changed" for changelog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants