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

Bubble init_send_tx error instead of unwrapping #600

Merged
merged 1 commit into from
Apr 7, 2021

Conversation

trevyn
Copy link
Contributor

@trevyn trevyn commented Apr 4, 2021

Fixes #582 "grin-wallet crashes when attempting to send amount larger than balance":

$ grin-wallet send -e -m 220.5
Password: 
Wallet command failed: LibWallet Error: Not enough funds. Required: 220.512000000, Available: 0.000000000
$

cargo test --all passes locally.

@trevyn
Copy link
Contributor Author

trevyn commented Apr 4, 2021

Oof, that's a weird one. Maybe triggered because something exited with an error where it used to panic. Will look into it.

Os { code: 5, kind: PermissionDenied, message: "Access is denied." }', controller\tests\common\mod.rs:80:38

pub fn clean_output_dir(test_dir: &str) {
let path = std::path::Path::new(test_dir);
if path.is_dir() {
fs::remove_dir_all(test_dir).unwrap();
}
}

---- wallet_self_send stdout ----
thread 'wallet_self_send' panicked at 'called `Result::unwrap()` on an `Err` value: Os { code: 5, kind: PermissionDenied, message: "Access is denied." }', controller\tests\common\mod.rs:80:38
stack backtrace:
   0: std::panicking::begin_panic_handler
             at /rustc/cb75ad5db02783e8b0222fee363c5f63f7e2cf5b\/library\std\src\panicking.rs:493
   1: core::panicking::panic_fmt
             at /rustc/cb75ad5db02783e8b0222fee363c5f63f7e2cf5b\/library\core\src\panicking.rs:92
   2: core::option::expect_none_failed
             at /rustc/cb75ad5db02783e8b0222fee363c5f63f7e2cf5b\/library\core\src\option.rs:1268
   3: core::result::Result<tuple<>, std::io::error::Error>::unwrap<tuple<>,std::io::error::Error>
             at /rustc/cb75ad5db02783e8b0222fee363c5f63f7e2cf5b\library\core\src\result.rs:973
   4: self_send::common::clean_output_dir
             at .\tests\common\mod.rs:80
   5: self_send::wallet_self_send
             at .\tests\self_send.rs:147
   6: self_send::wallet_self_send::{{closure}}
             at .\tests\self_send.rs:141

@trevyn
Copy link
Contributor Author

trevyn commented Apr 5, 2021

Ok, it looks like that Windows CI failure was spurious — there's a widely-used crate called remove_dir_all that mentions a possible cause:

// On Windows it is not enough to just recursively remove the contents of a
// directory and then the directory itself. Deleting does not happen
// instantaneously, but is delayed by IO being completed in the fs stack.
//
// Further, typical Windows machines can handle many more concurrent IOs
// than a single threaded application is capable of submitting: the
// overlapped (async) calls available do not cover the operations needed to
// perform directory removal.
  • We could use that crate for more robust handling
    or
  • We could ignore errors in fs::remove_dir_all (it's only cleaning up after tests)
    or
  • We can merge this as-is and leave the flaky test for later. :)

Copy link
Member

@quentinlesceller quentinlesceller left a comment

Choose a reason for hiding this comment

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

Looking good 👍 For the Windows CI we have indeed issues since a while. We can open an issue and track it.

@quentinlesceller
Copy link
Member

Should we go ahead and merge this @trevyn ?

@trevyn
Copy link
Contributor Author

trevyn commented Apr 7, 2021

@quentinlesceller Yep!

@quentinlesceller quentinlesceller merged commit 5189de5 into mimblewimble:master Apr 7, 2021
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.

grin-wallet crashes when attempting to send amount larger than balance
2 participants