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

Modify the method Relayer::accept_block to return StatusCode::BlockIsInvalid when shared.insert_new_block() produces an error. #4333

Conversation

eval-exec
Copy link
Collaborator

What problem does this PR solve?

Let Relayer::accept_block return Status

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)
  • No code ci-runs-only: [ quick_checks,linters ]

Side effects

  • None

Release note

None: Exclude this PR from the release note.

@eval-exec eval-exec requested a review from a team as a code owner January 30, 2024 04:11
@eval-exec eval-exec marked this pull request as draft January 30, 2024 04:50
@eval-exec eval-exec marked this pull request as ready for review January 30, 2024 04:55
@eval-exec eval-exec marked this pull request as draft January 30, 2024 06:36
@eval-exec
Copy link
Collaborator Author

eval-exec commented Jan 30, 2024

TODO:

  • Fix relayer::tests::compact_block_process::test_accept_block
  • Fix relayer::tests::compact_block_process::test_accept_not_a_better_block
        FAIL [   0.631s] ckb-sync relayer::tests::compact_block_process::test_accept_block

--- STDOUT:              ckb-sync relayer::tests::compact_block_process::test_accept_block ---

running 1 test
test relayer::tests::compact_block_process::test_accept_block ... FAILED

failures:

failures:
    relayer::tests::compact_block_process::test_accept_block

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 69 filtered out; finished in 0.61s


--- STDERR:              ckb-sync relayer::tests::compact_block_process::test_accept_block ---
thread 'relayer::tests::compact_block_process::test_accept_block' panicked at 'assertion failed: `(left == right)`
  left: `Status { code: BlockIsInvalid, context: Some("Byte32(0x7128196a2d588fa711b44cf2076ca26e1ee9be06060e67c5b260e5785a47615e), error: Block(Cellbase(InvalidQuantity))") }`,
 right: `Status { code: OK, context: None }`', sync/src/relayer/tests/compact_block_process.rs:392:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

        FAIL [   0.661s] ckb-sync relayer::tests::compact_block_process::test_accept_not_a_better_block

--- STDOUT:              ckb-sync relayer::tests::compact_block_process::test_accept_not_a_better_block ---

running 1 test
test relayer::tests::compact_block_process::test_accept_not_a_better_block ... FAILED

failures:

failures:
    relayer::tests::compact_block_process::test_accept_not_a_better_block

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 69 filtered out; finished in 0.64s


--- STDERR:              ckb-sync relayer::tests::compact_block_process::test_accept_not_a_better_block ---
thread 'relayer::tests::compact_block_process::test_accept_not_a_better_block' panicked at 'assertion failed: `(left == right)`
  left: `Status { code: BlockIsInvalid, context: Some("Byte32(0xd0176ece8f840c998f24aacdfcdccd29dca4a9a65d1f7cc860c09415853bbb7e), error: Block(Cellbase(InvalidQuantity))") }`,
 right: `Status { code: OK, context: None }`', sync/src/relayer/tests/compact_block_process.rs:185:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

@eval-exec
Copy link
Collaborator Author

The issue with the unit test test_accept_not_a_better_block has been resolved. However, the unit test test_accept_block remains challenging to fix...

@eval-exec eval-exec force-pushed the exec/relayer-handle-accept_block_error branch from a4d4db4 to 740f2f3 Compare January 31, 2024 13:13
@eval-exec eval-exec marked this pull request as ready for review January 31, 2024 13:14
@zhangsoledad zhangsoledad added this pull request to the merge queue Feb 1, 2024
Merged via the queue into nervosnetwork:develop with commit dfa4f37 Feb 1, 2024
33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants