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

flake: panic: Must deleted existing spendable note #5271

Closed
dpc opened this issue May 12, 2024 · 0 comments · Fixed by #5279
Closed

flake: panic: Must deleted existing spendable note #5271

dpc opened this issue May 12, 2024 · 0 comments · Fixed by #5279

Comments

@dpc
Copy link
Contributor

dpc commented May 12, 2024

00:08:31     thread 'tokio-runtime-worker' panicked at modules/fedimint-mint-client/src/lib.rs:1577:10:
00:08:31     Must deleted existing spendable note
00:08:31     stack backtrace:
00:08:31        0: rust_begin_unwind
00:08:31                  at /rustc/25ef9e3d85d934b27d9dada2f9dd52b1dc63bb04/library/std/src/panicking.rs:647:5
00:08:31        1: core::panicking::panic_fmt
00:08:31                  at /rustc/25ef9e3d85d934b27d9dada2f9dd52b1dc63bb04/library/core/src/panicking.rs:72:14
00:08:31        2: core::panicking::panic_display
00:08:31                  at /rustc/25ef9e3d85d934b27d9dada2f9dd52b1dc63bb04/library/core/src/panicking.rs:196:5
00:08:31        3: core::panicking::panic_str
00:08:31                  at /rustc/25ef9e3d85d934b27d9dada2f9dd52b1dc63bb04/library/core/src/panicking.rs:171:5
00:08:31        4: core::option::expect_failed
00:08:31                  at /rustc/25ef9e3d85d934b27d9dada2f9dd52b1dc63bb04/library/core/src/option.rs:1988:5
00:08:31        5: core::option::Option<T>::expect
00:08:31                  at /rustc/25ef9e3d85d934b27d9dada2f9dd52b1dc63bb04/library/core/src/option.rs:894:21
00:08:31        6: {async_fn#0}
00:08:31                  at ./modules/fedimint-mint-client/src/lib.rs:1572:9
00:08:31        7: {async_fn#0}
00:08:31                  at ./modules/fedimint-mint-client/src/lib.rs:1000:80
00:08:31        8: {async_block#0}
00:08:31                  at ./modules/fedimint-mint-client/src/lib.rs:663:56

https://github.com/fedimint/fedimint/actions/runs/9041274494/job/24860479614?pr=5259

dpc added a commit to dpc/fedimint that referenced this issue May 12, 2024
In one of the recent commits I've added an assertion to make sure a
spendable note was actually always removed when we thought it was,
as I was investigating a bug causing notes to be lost (fedimint#5195),
and it seemed like a good idea.

Too bad it causing flakes now: fedimint#5271

So it looks like our reliance on retrying db conflicts (`autocommit`),
leads to inability to assert basic expectations about database state,
which seems terrible.

In PR I'm adding a little lock to prevent write-write conflicts in this
area of the code, and I think it's worthwhile, just to be able to
ensure database integrity.

Moreover, I realized that `autocommit` is probably a bad idea altogether.

Application typically require transaction conflict handling, because
they are using **shared** databases. But with an embedded databases,
there is no sharing with external entities so there *always* should be,
or even *must be* a way to structure application and locking to avoid
any conflicts, preventing db isolation level issues, inability
to assert basic stuff, or just wasting time retrying things.
dpc added a commit to dpc/fedimint that referenced this issue May 13, 2024
dpc added a commit to dpc/fedimint that referenced this issue May 13, 2024
dpc added a commit to dpc/fedimint that referenced this issue May 13, 2024
dpc added a commit to dpc/fedimint that referenced this issue May 13, 2024
@dpc dpc closed this as completed in #5279 May 13, 2024
github-actions bot pushed a commit that referenced this issue May 14, 2024
github-actions bot pushed a commit that referenced this issue May 14, 2024
Fix #5271
Fix #5195

(cherry picked from commit 1d547a9)
okjodom pushed a commit to okjodom/fedimint that referenced this issue Jun 7, 2024
okjodom pushed a commit to okjodom/fedimint that referenced this issue Jun 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant