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

Better error handling during UTXO Scan #285

Merged
merged 1 commit into from
Jan 2, 2020

Conversation

yeastplume
Copy link
Member

As reported here https://www.grin-forum.org/t/wallet-error-when-checking-balance/6840/2 and on keybase, older chain data appears to be pulling outputs directly from the utxo set but returning them as spent. Not attempting any logic changes this close to release, but this removes the unwraps mentioned in the trace below and should log full information about the error when it occurs. Conversation below for reference.

There seems to be something odd going on in older node data... the stacktrace reported in the ecosystem channel is as follows: 
thread 'main' panicked at 'called `Option::unwrap()` on a `None` value': src/libcore/option.rs:378stack backtrace:
   0: backtrace::backtrace::trace
   1: backtrace::capture::Backtrace::new
   2: grin_util::logger::send_panic_to_log::{{closure}}
   3: std::panicking::rust_panic_with_hook
   4: std::panicking::continue_panic_fmt
   5: rust_begin_unwind
   6: core::panicking::panic_fmt
   7: core::panicking::panic
   8: <grin_wallet_impls::node_clients::http::HTTPNodeClient as grin_wallet_libwallet::types::NodeClient>::get_outputs_by_pmmr_index
   9: grin_wallet_libwallet::internal::scan::scan
  10: grin_wallet_libwallet::api_impl::owner::scan
  11: grin_wallet_api::owner::Owner<L,C,K>::scan
  12: grin_wallet_controller::controller::owner_single_use
  13: grin_wallet_controller::command::scan
  14: grin_wallet::cmd::wallet_args::wallet_command
  15: grin_wallet::cmd::wallet::wallet_command
  16: grin_wallet::real_main
  17: grin_wallet::main
  18: std::rt::lang_start::{{closure}}
  19: std::panicking::try::do_call
  20: __rust_maybe_catch_panic
  21: std::rt::lang_start_internal
  22: main
Thread 'main' panicked with message:
"called `Option::unwrap()` on a `None` value"
And that happens during a wallet scan. All that's happening at this point is that the wallet is telling the node to return the outputs in its utxo set (it's not requesting any particular outputs explicitly)
That panic on unwrap can only be triggered by the block_height unwrap here https://github.com/mimblewimble/grin-wallet/blob/master/impls/src/node_clients/http.rs#L269, and that can only be None if the output is spent: https://github.com/mimblewimble/grin/blob/2bf4080866da89cddb86032e1b7ffa01af503192/api/src/types.rs#L295
So it would seem the node is somehow pulling an output from the MMR then deciding it's spent in the same operation, which doesn't make a lot of sense
That NotFound during the unspent check also seems to be being triggered here https://www.grin-forum.org/t/wallet-error-when-checking-balance/6840
At this point in the unspent check https://github.com/mimblewimble/grin/blob/2bf4080866da89cddb86032e1b7ffa01af503192/chain/src/txhashset/txhashset.rs#L247
tromp
11:49 AM
what is the output in question?
yeastplume
11:50 AM
I don't have that info, this is from user reports so this is mostly deduction
tromp
11:51 AM
oh, so it's not reproducible on your end
yeastplume
11:51 AM
No, but there's more than one report of it (see ecosystem channel and forum question)
tromp
11:51 AM
might be a corrupted database
yeastplume
11:51 AM
Indeed, it's fixable by resyncing the node, it would seem
tromp
11:52 AM
one prudent thing to do is to replace unwrap by a check that prints an error message including as much info as possible
yeastplume
11:53 AM
Yes, I think that's the only change I'm going to make before 3.0.0 release (there shouldn't be any unwraps in there in the first place)

@yeastplume yeastplume merged commit cf47c1d into mimblewimble:current/3.0.x Jan 2, 2020
yeastplume added a commit to yeastplume/grin-wallet that referenced this pull request Jan 2, 2020
@yeastplume yeastplume mentioned this pull request Jan 2, 2020
yeastplume added a commit that referenced this pull request Jan 2, 2020
@yeastplume yeastplume deleted the 3.0.0-unwraps branch January 6, 2020 09:56
yyangli pushed a commit to mwcproject/mwc-wallet that referenced this pull request May 13, 2020
antiochp pushed a commit to antiochp/grin-wallet that referenced this pull request Aug 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant