talaia-labs / rust-teos

The Eye of Satoshi - Lightning Watchtower
https://talaia-labs.github.io/talaia.watch/
MIT License
134 stars 63 forks source link

Blockchain::at_height may return invalid data #56

Closed sr-gi closed 2 years ago

sr-gi commented 2 years ago

Blockchain::at_height may pull a BlockHeader that cannot be verified. This have been seen in CI in rare occasions.

Not a big deal, but it may be nice to find the root of the error so tests do not fail randomly on CI.

---- watcher::tests::test_cache_fix stdout ----
thread 'watcher::tests::test_cache_fix' panicked at 'called `Result::unwrap()` on an `Err` value: BlockSourceError { kind: Persistent, error: BlockBadProofOfWork }', teos/src/test_utils.rs:163:43
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

failures:
    watcher::tests::test_cache_fix

test result: FAILED. 149 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 25.68s
mariocynicys commented 2 years ago

I did try running the test till it fails. image For some generated blocks, it happens that the block hash is greater than the target (0xffff), this happens very rarely though.

A possible fix is to assure that all blockhashes are <= the constant target at block generation time (here). So if we face a block with a small hash, we discard it and repeat the process with different txs (or shuffle the txs).

sr-gi commented 2 years ago

I'm reopening this since we may have missed and edge case. An error regarding BlockBadProofOfWork was recently seen on CI:

failures:

---- api::internal::tests_private_api::test_get_users stdout ----
thread 'api::internal::tests_private_api::test_get_users' panicked at 'called `Result::unwrap()` on an `Err` value: BlockSourceError { kind: Persistent, error: BlockBadProofOfWork }', teos/src/test_utils.rs:381:14
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

failures:
    api::internal::tests_private_api::test_get_users

Ref -> https://github.com/talaia-labs/rust-teos/actions/runs/3051585175/jobs/4919990408#step:5:697-699

mariocynicys commented 2 years ago

Lol Idk why I was surprised even though I faced this issue some weeks ago xD I have a commit that fixes it, but didn't PR that branch yet.

sr-gi commented 2 years ago

Not sure if that's part of a bigger PR, but I think that'd do as a standalone one.

I'd suggest to cherry-pick it and send it.