Skip to content

Conversation

Quexington
Copy link
Contributor

This PR adds support for Revocable CATs to the wallet. It ended up being a thin modification of the base CATWallet class via inheritance.

In the tests, there is an unfortunate addition of an Any which is sort of necessary because of how the method it's relevant to needs to be refactored.

@Quexington Quexington added the Added Required label for PR that categorizes merge commit message as "Added" for changelog label Jul 9, 2025
Copy link

coveralls-official bot commented Jul 9, 2025

Pull Request Test Coverage Report for Build 16204290850

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 304 of 353 (86.12%) changed or added relevant lines in 22 files are covered.
  • 62 unchanged lines in 15 files lost coverage.
  • Overall coverage decreased (-0.02%) to 91.365%

Changes Missing Coverage Covered Lines Changed/Added Lines %
chia/wallet/nft_wallet/ownership_outer_puzzle.py 2 3 66.67%
chia/wallet/vc_wallet/vc_drivers.py 21 23 91.3%
chia/wallet/cat_wallet/lineage_store.py 1 6 16.67%
chia/wallet/wallet_state_manager.py 28 39 71.79%
chia/wallet/cat_wallet/r_cat_wallet.py 110 140 78.57%
Files with Coverage Reduction New Missed Lines %
chia/daemon/keychain_proxy.py 1 73.24%
chia/farmer/farmer.py 1 73.28%
chia/full_node/pending_tx_cache.py 1 96.55%
chia/rpc/rpc_server.py 1 88.24%
chia/_tests/core/util/test_lockfile.py 1 90.74%
chia/wallet/nft_wallet/transfer_program_puzzle.py 1 88.89%
chia/wallet/wallet_node.py 1 87.67%
chia/daemon/client.py 2 74.16%
chia/full_node/coin_store.py 3 99.11%
chia/server/node_discovery.py 4 81.75%
Totals Coverage Status
Change from base Build 16149582872: -0.02%
Covered Lines: 102381
Relevant Lines: 111935

💛 - Coveralls

@Quexington Quexington marked this pull request as ready for review July 9, 2025 14:21
@Quexington Quexington requested a review from a team as a code owner July 9, 2025 14:21
@emlowe emlowe requested a review from Copilot July 9, 2025 23:22
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Adds support for Revocable CATs by introducing a new RCATWallet subclass and integrating it across wallet state management, puzzle drivers, and tests.

  • Introduce WalletType.RCAT and register RCATWallet in state manager and asset‐id lookups
  • Implement RevocationOuterPuzzle driver and extend get_inner_puzzle signatures project‐wide
  • Parameterize existing CAT tests to run against both CATWallet and RCATWallet

Reviewed Changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated no comments.

Show a summary per file
File Description
chia/wallet/wallet_state_manager.py Import and handle RCATWallet in creation, handle_cat, and get_wallet_for_asset_id
chia/wallet/vc_wallet/vc_drivers.py Add RevocationOuterPuzzle with new match/construct/solve logic
chia/wallet/outer_puzzles.py Register RevocationOuterPuzzle under new AssetType.REVOCATION_LAYER
chia/wallet/driver_protocol.py Update get_inner_puzzle signature to accept optional solution argument
chia/wallet/cat_wallet/.py & chia/wallet/vc_wallet/.py & nft_wallet/*.py Update driver method signatures (get_inner_puzzle) to include solution: Optional[Program]
chia/wallet/cat_wallet/r_cat_wallet.py New RCATWallet implementation inheriting from CATWallet
chia/_tests/wallet/cat_wallet/*.py Parameterize tests to run for both CATWallet and RCATWallet
Comments suppressed due to low confidence (5)

chia/wallet/wallet_state_manager.py:331

  • The RCAT creation branch is marked # pragma: no cover, so it isn't exercised by tests—either add tests for this path or remove the pragma.
            elif wallet_type == WalletType.RCAT:  # pragma: no cover

chia/wallet/driver_protocol.py:16

  • [nitpick] Consider giving the solution parameter a default of None in the Protocol signature so that implementations with an optional solution parameter align cleanly.
    def get_inner_puzzle(

chia/wallet/wallet_state_manager.py:1138

  • [nitpick] Remove placeholder debug logs (HERE, HERE2, etc.) or replace them with meaningful log messages at an appropriate log level.
            self.log.error(f"HERE")

chia/wallet/vc_wallet/vc_drivers.py:26

  • Import Any from the standard typing module instead of typing_extensions to avoid import errors; keep Self from typing_extensions if needed.
from typing_extensions import Any, Self

chia/wallet/trading/offer.py:238

  • Ensure that parent_solution is defined in this scope (or correctly named) before passing it to get_inner_puzzle, otherwise this will raise a NameError.
                inner_puzzle: Optional[Program] = get_inner_puzzle(puzzle_driver, parent_puzzle, parent_solution)

Copy link
Contributor

@altendky altendky left a comment

Choose a reason for hiding this comment

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

here's a quick pass without digging into an understanding of the way the revocable cat works or details around any special wallet tracking needs or... i did diff the new subclass against the parent to compare what was overridden and that it looked kinda ok'ish.

for the less localized inheritance related tweaks, i expect to withdraw them. but, i had already typed them up so will submit them and we can let you see some possibilities here, then i'll close them.

Quexington

This comment was marked as spam.

Copy link
Contributor

Copy link
Contributor

@emlowe emlowe left a comment

Choose a reason for hiding this comment

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

Code coverage exemption

@cmmarslender cmmarslender merged commit b9710b1 into main Jul 10, 2025
522 of 525 checks passed
@cmmarslender cmmarslender deleted the quex.add_r_cats_to_wallet branch July 10, 2025 20:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants