Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clawback resync #15496

Merged
merged 24 commits into from
Jul 11, 2023
Merged

Clawback resync #15496

merged 24 commits into from
Jul 11, 2023

Conversation

ytx1991
Copy link
Contributor

@ytx1991 ytx1991 commented Jun 13, 2023

Purpose:

Current Behavior:

New Behavior:

Testing Notes:

@ytx1991 ytx1991 changed the base branch from main to release/1.8.2 June 13, 2023 05:21
@ytx1991 ytx1991 changed the base branch from release/1.8.2 to main June 13, 2023 05:25
@ytx1991 ytx1991 changed the base branch from main to release/1.8.2 June 13, 2023 05:25
@ytx1991 ytx1991 marked this pull request as ready for review June 13, 2023 19:12
@ytx1991 ytx1991 requested a review from a team as a code owner June 13, 2023 19:12
@xdustinface
Copy link
Contributor

Reopening because CI obviously only run conflicts checks here.

@xdustinface xdustinface reopened this Jun 13, 2023
@ytx1991 ytx1991 added the Fixed Required label for PR that categorizes merge commit message as "Fixed" for changelog label Jun 13, 2023
chia/wallet/wallet_state_manager.py Outdated Show resolved Hide resolved
@ytx1991 ytx1991 closed this Jun 16, 2023
@ytx1991
Copy link
Contributor Author

ytx1991 commented Jun 16, 2023

This fix is not necessary for the release, close for now

@ytx1991 ytx1991 reopened this Jun 19, 2023
@xdustinface
Copy link
Contributor

@ytx1991 The test you added in 91ac933 doesn't seem to pass. It still running on CI but times out here locally with the following:

21:02:20 chia.wallet.wallet_node: INFO Selected multiprocessing start method: default
21:02:20 chia.wallet.wallet_node: INFO Resetting wallet sync data...
21:02:20 chia.wallet.wallet_node: INFO Reset wallet sync data completed.
21:02:20 chia.wallet.wallet_node: DEBUG Trying to disable resync: 2999502625 [2999502625]
21:02:20 chia.wallet.wallet_node: INFO Disabled resync for wallet fingerprint: 2999502625
21:02:20 chia.wallet.wallet_state_manager: DEBUG Starting in db path: /private/var/folders/6b/sb101ygd0_xfxmbc_ljxybr80000gn/T/tmprryrmaxf/test-wallet-db-None-2999502625.sqlite
21:02:20 chia.wallet.wallet_node: INFO Wallet is logged in using key with fingerprint: 2999502625
21:02:20 chia.wallet.wallet_node: INFO Updated last used fingerprint: 2999502625
21:02:20 chia.wallet.did_wallet.did_wallet: INFO Confirmed balance for did wallet is 0
21:02:20 chia.wallet.did_wallet.did_wallet: INFO Confirmed balance for did wallet is 0
FAILED
tests/wallet/rpc/test_wallet_rpc.py:1907 (test_fully_resync[True])
tests/wallet/rpc/test_wallet_rpc.py:1980: in test_fully_resync
    await time_out_assert(20, wc2.get_synced)
E   AssertionError: Timed assertion timed out after 22 seconds: expected True, got False

@wallentx wallentx closed this Jun 20, 2023
@wallentx wallentx reopened this Jun 20, 2023
@ytx1991 ytx1991 requested a review from xdustinface June 26, 2023 15:52
@wallentx wallentx changed the base branch from release/1.8.2 to main June 27, 2023 15:09
@ytx1991 ytx1991 requested a review from xdustinface June 28, 2023 16:49
Copy link
Contributor

@xdustinface xdustinface left a comment

Choose a reason for hiding this comment

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

There is still a leftover from the retargeting to main and two nits.

.github/workflows/test.yml Outdated Show resolved Hide resolved
chia/wallet/wallet_state_manager.py Outdated Show resolved Hide resolved
tests/wallet/rpc/test_wallet_rpc.py Outdated Show resolved Hide resolved
xdustinface
xdustinface previously approved these changes Jun 29, 2023
chia/wallet/wallet_state_manager.py Show resolved Hide resolved
Copy link
Contributor

@danieljperry danieljperry left a comment

Choose a reason for hiding this comment

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

I cloned main and added this PR. I'm having trouble getting auto claim to work. This video shows when it fails (I did cut out a lot where nothing was happening, but I didn't cut the auto claim part):

Clawback.Autoclaim.main+15496.mp4

Here's my log from when the claim happened:

auto_claim.txt

@danieljperry
Copy link
Contributor

Here's another issue I saw while attempting to claw back a transaction. Note that this problem might not be related to this PR:

nonetype.mp4

Log snippet:

Traceback (most recent call last):
  File "C:\Users\User\Chia\main+15496\chia-blockchain\chia\rpc\wallet_rpc_api.py", line 912, in get_transactions
    assert record is not None, f"Cannot find coin record for clawback transaction {tx['name']}"
AssertionError: Cannot find coin record for clawback transaction 0x1b332cee62521ffe34d04712db291eeb3681e2453ff50811d65d0b87c9d470c3
2023-06-29T11:34:22.696 wallet chia.rpc.rpc_server        : WARNING  Error while handling message: Traceback (most recent call last):
  File "C:\Users\User\Chia\main+15496\chia-blockchain\chia\rpc\rpc_server.py", line 340, in safe_handle
    response = await self.ws_api(message)
  File "C:\Users\User\Chia\main+15496\chia-blockchain\chia\rpc\rpc_server.py", line 331, in ws_api
    return await f_rpc_api(data)
  File "C:\Users\User\Chia\main+15496\chia-blockchain\chia\rpc\wallet_rpc_api.py", line 1062, in spend_clawback_coins
    coin_ids: List[bytes32] = [bytes32.from_hexstr(coin) for coin in request["coin_ids"]]
  File "C:\Users\User\Chia\main+15496\chia-blockchain\chia\rpc\wallet_rpc_api.py", line 1062, in <listcomp>
    coin_ids: List[bytes32] = [bytes32.from_hexstr(coin) for coin in request["coin_ids"]]
  File "C:\Users\User\Chia\main+15496\chia-blockchain\chia\util\byte_types.py", line 51, in from_hexstr
    if input_str.startswith("0x") or input_str.startswith("0X"):
AttributeError: 'NoneType' object has no attribute 'startswith'```

@ytx1991
Copy link
Contributor Author

ytx1991 commented Jun 29, 2023

Here's another issue I saw while attempting to claw back a transaction. Note that this problem might not be related to this PR:

nonetype.mp4
Log snippet:

Traceback (most recent call last):
  File "C:\Users\User\Chia\main+15496\chia-blockchain\chia\rpc\wallet_rpc_api.py", line 912, in get_transactions
    assert record is not None, f"Cannot find coin record for clawback transaction {tx['name']}"
AssertionError: Cannot find coin record for clawback transaction 0x1b332cee62521ffe34d04712db291eeb3681e2453ff50811d65d0b87c9d470c3
2023-06-29T11:34:22.696 wallet chia.rpc.rpc_server        : WARNING  Error while handling message: Traceback (most recent call last):
  File "C:\Users\User\Chia\main+15496\chia-blockchain\chia\rpc\rpc_server.py", line 340, in safe_handle
    response = await self.ws_api(message)
  File "C:\Users\User\Chia\main+15496\chia-blockchain\chia\rpc\rpc_server.py", line 331, in ws_api
    return await f_rpc_api(data)
  File "C:\Users\User\Chia\main+15496\chia-blockchain\chia\rpc\wallet_rpc_api.py", line 1062, in spend_clawback_coins
    coin_ids: List[bytes32] = [bytes32.from_hexstr(coin) for coin in request["coin_ids"]]
  File "C:\Users\User\Chia\main+15496\chia-blockchain\chia\rpc\wallet_rpc_api.py", line 1062, in <listcomp>
    coin_ids: List[bytes32] = [bytes32.from_hexstr(coin) for coin in request["coin_ids"]]
  File "C:\Users\User\Chia\main+15496\chia-blockchain\chia\util\byte_types.py", line 51, in from_hexstr
    if input_str.startswith("0x") or input_str.startswith("0X"):
AttributeError: 'NoneType' object has no attribute 'startswith'```

It seems like the clawback coin is missing for some reason. I assume it is related to wallet switching. For the auto claim, did you wait several minutes after the timelock expired?

@danieljperry
Copy link
Contributor

Here's another issue I saw while attempting to claw back a transaction. Note that this problem might not be related to this PR:
nonetype.mp4
Log snippet:

Traceback (most recent call last):
  File "C:\Users\User\Chia\main+15496\chia-blockchain\chia\rpc\wallet_rpc_api.py", line 912, in get_transactions
    assert record is not None, f"Cannot find coin record for clawback transaction {tx['name']}"
AssertionError: Cannot find coin record for clawback transaction 0x1b332cee62521ffe34d04712db291eeb3681e2453ff50811d65d0b87c9d470c3
2023-06-29T11:34:22.696 wallet chia.rpc.rpc_server        : WARNING  Error while handling message: Traceback (most recent call last):
  File "C:\Users\User\Chia\main+15496\chia-blockchain\chia\rpc\rpc_server.py", line 340, in safe_handle
    response = await self.ws_api(message)
  File "C:\Users\User\Chia\main+15496\chia-blockchain\chia\rpc\rpc_server.py", line 331, in ws_api
    return await f_rpc_api(data)
  File "C:\Users\User\Chia\main+15496\chia-blockchain\chia\rpc\wallet_rpc_api.py", line 1062, in spend_clawback_coins
    coin_ids: List[bytes32] = [bytes32.from_hexstr(coin) for coin in request["coin_ids"]]
  File "C:\Users\User\Chia\main+15496\chia-blockchain\chia\rpc\wallet_rpc_api.py", line 1062, in <listcomp>
    coin_ids: List[bytes32] = [bytes32.from_hexstr(coin) for coin in request["coin_ids"]]
  File "C:\Users\User\Chia\main+15496\chia-blockchain\chia\util\byte_types.py", line 51, in from_hexstr
    if input_str.startswith("0x") or input_str.startswith("0X"):
AttributeError: 'NoneType' object has no attribute 'startswith'```

It seems like the clawback coin is missing for some reason. I assume it is related to wallet switching. For the auto claim, did you wait several minutes after the timelock expired?

In this case for autoclaim, I switched to the receiver's wallet before the timelock expired. The autoclaim was then attempted in that wallet automatically as soon as the timelock expired.

@ytx1991 ytx1991 added the ready_to_merge Submitter and reviewers think this is ready label Jul 6, 2023
@github-actions github-actions bot added the merge_conflict Branch has conflicts that prevent merge to main label Jul 7, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jul 7, 2023

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot removed the merge_conflict Branch has conflicts that prevent merge to main label Jul 10, 2023
@github-actions
Copy link
Contributor

Conflicts have been resolved. A maintainer will review the pull request shortly.

Copy link
Contributor

@wjblanke wjblanke left a comment

Choose a reason for hiding this comment

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

aok

@wallentx wallentx merged commit 40a347f into main Jul 11, 2023
203 checks passed
@wallentx wallentx deleted the clawback_resync branch July 11, 2023 18:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Fixed Required label for PR that categorizes merge commit message as "Fixed" for changelog ready_to_merge Submitter and reviewers think this is ready
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants