Skip to content

Commit

Permalink
[CHIA-712] Fix CATWallet pending_change calculation (#18126)
Browse files Browse the repository at this point in the history 
* Add the concept of 'action scopes'

* Add `WalletActionScope`

* Fix CATWallet pending_change calculation

* Add the concept of 'action scopes'

* pylint and test coverage

* add try/finally

* add try/except

* Undo giving a variable a name

* Fix CRCAT test

* Fix trade tests

* Fix cat test
  • Loading branch information
@Quexington
Quexington authored Jul 11, 2024
1 parent f41a494 commit d6761cd
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 4 deletions.
4 changes: 2 additions & 2 deletions chia/_tests/wallet/cat_wallet/test_cat_wallet.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ async def test_cat_spend(wallet_environments: WalletTestFramework) -> None:
"confirmed_wallet_balance": 0,
"unconfirmed_wallet_balance": 100,
"spendable_balance": 0,
"pending_change": 0,
"pending_change": 100, # A little weird but technically correct
"max_send_amount": 0,
"unspent_coin_count": 0,
"pending_coin_removal_count": 1, # The ephemeral eve spend
Expand All @@ -226,7 +226,7 @@ async def test_cat_spend(wallet_environments: WalletTestFramework) -> None:
"cat": {
"confirmed_wallet_balance": 100,
"spendable_balance": 100,
"pending_change": 0,
"pending_change": -100,
"max_send_amount": 100,
"unspent_coin_count": 1,
"pending_coin_removal_count": -1,
Expand Down
21 changes: 21 additions & 0 deletions chia/_tests/wallet/cat_wallet/test_trades.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -515,6 +515,7 @@ async def test_cat_trades(
"unconfirmed_wallet_balance": 0,
"spendable_balance": 0,
"max_send_amount": 0,
"pending_change": 0,
"unspent_coin_count": 0,
}
if credential_restricted
Expand Down Expand Up @@ -550,6 +551,7 @@ async def test_cat_trades(
"new cat": {
"unconfirmed_wallet_balance": -2,
"pending_coin_removal_count": 1,
"pending_change": 98,
"<=#spendable_balance": -2,
"<=#max_send_amount": -2,
},
Expand All @@ -576,6 +578,7 @@ async def test_cat_trades(
"new cat": {
"confirmed_wallet_balance": -2,
"pending_coin_removal_count": -1,
"pending_change": -98,
">#spendable_balance": 0,
">#max_send_amount": 0,
},
Expand Down Expand Up @@ -607,6 +610,7 @@ async def test_cat_trades(
"new cat": {
"unconfirmed_wallet_balance": 2,
"pending_coin_removal_count": 1,
"pending_change": 2, # This is a little weird but fits the current definition
},
"vc": {
"pending_coin_removal_count": 1,
Expand All @@ -622,6 +626,7 @@ async def test_cat_trades(
"confirmed_wallet_balance": 2,
"spendable_balance": 2,
"max_send_amount": 2,
"pending_change": -2,
"unspent_coin_count": 1,
"pending_coin_removal_count": -1,
},
Expand Down Expand Up @@ -889,6 +894,7 @@ async def assert_trade_tx_number(wallet_node: WalletNode, trade_id: bytes32, num
"new cat": {
"unconfirmed_wallet_balance": -6,
"<=#spendable_balance": -6,
"pending_change": 92,
"<=#max_send_amount": -6,
"pending_coin_removal_count": 1,
},
Expand All @@ -911,6 +917,7 @@ async def assert_trade_tx_number(wallet_node: WalletNode, trade_id: bytes32, num
},
"new cat": {
"confirmed_wallet_balance": -6,
"pending_change": -92,
">#spendable_balance": 0,
">#max_send_amount": 0,
"pending_coin_removal_count": -1,
Expand Down Expand Up @@ -946,6 +953,7 @@ async def assert_trade_tx_number(wallet_node: WalletNode, trade_id: bytes32, num
"new cat": {
"unconfirmed_wallet_balance": 6,
"pending_coin_removal_count": 1,
"pending_change": 6, # This is a little weird but fits the current definition
},
"vc": {
"pending_coin_removal_count": 1,
Expand All @@ -961,6 +969,7 @@ async def assert_trade_tx_number(wallet_node: WalletNode, trade_id: bytes32, num
"confirmed_wallet_balance": 6,
"spendable_balance": 6,
"max_send_amount": 6,
"pending_change": -6,
"unspent_coin_count": 1,
"pending_coin_removal_count": -1,
},
Expand Down Expand Up @@ -1109,12 +1118,14 @@ async def assert_trade_tx_number(wallet_node: WalletNode, trade_id: bytes32, num
"unconfirmed_wallet_balance": -8,
"<=#spendable_balance": -8,
"<=#max_send_amount": -8,
"pending_change": 1,
"pending_coin_removal_count": 2, # For the first time, we're using two coins in an offer
},
"new cat": {
"unconfirmed_wallet_balance": -9,
"<=#spendable_balance": -9,
"<=#max_send_amount": -9,
"pending_change": 83,
"pending_coin_removal_count": 1,
},
**(
Expand All @@ -1138,13 +1149,15 @@ async def assert_trade_tx_number(wallet_node: WalletNode, trade_id: bytes32, num
"confirmed_wallet_balance": -8,
">#spendable_balance": 0,
">#max_send_amount": 0,
"pending_change": -1,
"pending_coin_removal_count": -2,
"unspent_coin_count": -1,
},
"new cat": {
"confirmed_wallet_balance": -9,
">#spendable_balance": 0,
">#max_send_amount": 0,
"pending_change": -83,
"pending_coin_removal_count": -1,
},
**(
Expand Down Expand Up @@ -1178,6 +1191,7 @@ async def assert_trade_tx_number(wallet_node: WalletNode, trade_id: bytes32, num
"cat": {
"unconfirmed_wallet_balance": 8,
"pending_coin_removal_count": 1,
"pending_change": 8, # This is a little weird but fits the current definition
},
"vc": {
"pending_coin_removal_count": 1,
Expand All @@ -1193,6 +1207,7 @@ async def assert_trade_tx_number(wallet_node: WalletNode, trade_id: bytes32, num
"confirmed_wallet_balance": 8,
"spendable_balance": 8,
"max_send_amount": 8,
"pending_change": -8,
"unspent_coin_count": 1,
"pending_coin_removal_count": -1,
},
Expand Down Expand Up @@ -1223,6 +1238,7 @@ async def assert_trade_tx_number(wallet_node: WalletNode, trade_id: bytes32, num
"new cat": {
"unconfirmed_wallet_balance": 9,
"pending_coin_removal_count": 1,
"pending_change": 9, # This is a little weird but fits the current definition
},
"vc": {
"pending_coin_removal_count": 1,
Expand All @@ -1238,6 +1254,7 @@ async def assert_trade_tx_number(wallet_node: WalletNode, trade_id: bytes32, num
"confirmed_wallet_balance": 9,
"spendable_balance": 9,
"max_send_amount": 9,
"pending_change": -9,
"unspent_coin_count": 1,
"pending_coin_removal_count": -1,
},
Expand Down Expand Up @@ -1480,6 +1497,7 @@ async def assert_trade_tx_number(wallet_node: WalletNode, trade_id: bytes32, num
"unconfirmed_wallet_balance": -15,
"<=#spendable_balance": -15,
"<=#max_send_amount": -15,
"pending_change": 68,
"pending_coin_removal_count": 1,
},
**(
Expand Down Expand Up @@ -1509,6 +1527,7 @@ async def assert_trade_tx_number(wallet_node: WalletNode, trade_id: bytes32, num
"confirmed_wallet_balance": -15,
">#spendable_balance": 0,
">#max_send_amount": 0,
"pending_change": -68,
"pending_coin_removal_count": -1,
},
**(
Expand Down Expand Up @@ -1542,6 +1561,7 @@ async def assert_trade_tx_number(wallet_node: WalletNode, trade_id: bytes32, num
"new cat": {
"unconfirmed_wallet_balance": 15,
"pending_coin_removal_count": 1,
"pending_change": 15, # This is a little weird but fits the current definition
},
"vc": {
"pending_coin_removal_count": 1,
Expand All @@ -1557,6 +1577,7 @@ async def assert_trade_tx_number(wallet_node: WalletNode, trade_id: bytes32, num
"confirmed_wallet_balance": 15,
"spendable_balance": 15,
"max_send_amount": 15,
"pending_change": -15,
"unspent_coin_count": 1,
"pending_coin_removal_count": -1,
},
Expand Down
6 changes: 6 additions & 0 deletions chia/_tests/wallet/vc_wallet/test_vc_wallet.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,7 @@ async def test_vc_lifecycle(wallet_environments: WalletTestFramework) -> None:
"unconfirmed_wallet_balance": -90,
"spendable_balance": -100,
"max_send_amount": -100,
"pending_change": 10,
"pending_coin_removal_count": 1,
},
},
Expand All @@ -393,6 +394,7 @@ async def test_vc_lifecycle(wallet_environments: WalletTestFramework) -> None:
"confirmed_wallet_balance": -90,
"spendable_balance": 10,
"max_send_amount": 10,
"pending_change": -10,
"pending_coin_removal_count": -1,
},
},
Expand Down Expand Up @@ -482,6 +484,7 @@ async def test_vc_lifecycle(wallet_environments: WalletTestFramework) -> None:
},
"crcat": {
"unconfirmed_wallet_balance": 90,
"pending_change": 90,
"pending_coin_removal_count": 1,
},
},
Expand All @@ -498,6 +501,7 @@ async def test_vc_lifecycle(wallet_environments: WalletTestFramework) -> None:
"confirmed_wallet_balance": 90,
"spendable_balance": 90,
"max_send_amount": 90,
"pending_change": -90,
"unspent_coin_count": 1,
"pending_coin_removal_count": -1,
},
Expand Down Expand Up @@ -551,6 +555,7 @@ async def test_vc_lifecycle(wallet_environments: WalletTestFramework) -> None:
"unconfirmed_wallet_balance": -50,
"spendable_balance": -90,
"max_send_amount": -90,
"pending_change": 40,
"pending_coin_removal_count": 1,
},
},
Expand All @@ -567,6 +572,7 @@ async def test_vc_lifecycle(wallet_environments: WalletTestFramework) -> None:
"confirmed_wallet_balance": -50, # should go straight to confirmed because we sent to ourselves
"spendable_balance": 40,
"max_send_amount": 40,
"pending_change": -40,
"pending_coin_removal_count": -1,
"unspent_coin_count": 1,
},
Expand Down
9 changes: 7 additions & 2 deletions chia/wallet/cat_wallet/cat_wallet.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -472,7 +472,7 @@ async def get_pending_change_balance(self) -> uint64:
unconfirmed_tx = await self.wallet_state_manager.tx_store.get_unconfirmed_for_wallet(self.id())
addition_amount = 0
for record in unconfirmed_tx:
if not record.is_in_mempool():
if not record.is_in_mempool() and record.spend_bundle is not None:
continue
our_spend = False
for coin in record.removals:
Expand All @@ -484,7 +484,12 @@ async def get_pending_change_balance(self) -> uint64:
continue

for coin in record.additions:
if await self.wallet_state_manager.does_coin_belong_to_wallet(coin, self.id()):
hint_dict = {
coin_id: bytes32(memos[0])
for coin_id, memos in record.memos
if len(memos) > 0 and len(memos[0]) == 32
}
if await self.wallet_state_manager.does_coin_belong_to_wallet(coin, self.id(), hint_dict=hint_dict):
addition_amount += coin.amount

return uint64(addition_amount)
Expand Down

0 comments on commit d6761cd

Please sign in to comment.