-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Remove extra check in send_transaction_multi for chia coins split #16061
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't feel like the fix for the problem and it anyway will still run into the same issue in cat_spend
here
chia-blockchain/chia/rpc/wallet_rpc_api.py
Line 1071 in 7557b6c
transaction: Dict = (await self.cat_spend(request, hold_lock=False))["transaction"] |
which does
chia-blockchain/chia/rpc/wallet_rpc_api.py
Lines 1539 to 1540 in 7557b6c
if await self.service.wallet_state_manager.synced() is False: | |
raise ValueError("Wallet needs to be fully synced.") |
I think we should instead make sure WalletStateManager.synced
reliably knows when to return True
and my guess is that the issue is related to the queue not being empty when the requests are triggered. Maybe because it in wallet only mode in those cases it uses an untrusted node which does the secondary sync always and keeps items in the queue or so.
chia-blockchain/chia/wallet/wallet_state_manager.py
Lines 576 to 579 in 7557b6c
has_pending_queue_items = self.wallet_node.new_peak_queue.has_pending_data_process_items() | |
if latest_timestamp > int(time.time()) - 5 * 60 and not has_pending_queue_items: | |
return True |
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
It does look like one could also remove the check from So I think removing this check is ok, but you do need to remove it from It is a much deeper question as to understanding how the wallet is synced or not |
This PR has been flagged as stale due to no activity for over 60 days. It will not be automatically closed, but it has been given a stale-pr label and should be manually reviewed by the relevant parties. |
Purpose:
This removes a sync check from the wallet rpc
send_transaction_multi
Current Behavior:
N/A
New Behavior:
N/A
Testing Notes:
Applicable test coverage exists, however this issue is hard to replicate and just happens, and this is the best way to fix it