change: Improve CommitBlockError message by including underlying error message

[dimxy]

Motivation

Improve logging of CommitBlockError logged with the default Display method by extending with the underlying ValidateContextError message.

For example, before this fix a CommitBlockError with a ValidateContextError::Orphaned error would be printed as:

block is not contextually valid

with this fix:

block is not contextually valid: block height 12 is lower than the current finalized height 127

fixes #6218

[codecov]

Codecov Report

Merging #6236 (b28352f) into main (98c634b) will increase coverage by 0.10%.
The diff coverage is n/a.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6236      +/-   ##
==========================================
+ Coverage   77.83%   77.94%   +0.10%     
==========================================
  Files         304      304              
  Lines       39349    39349              
==========================================
+ Hits        30626    30669      +43     
+ Misses       8723     8680      -43     

[upbqdn]

This PR looks good, thank you.

Edit: I moved my question into the conversation below.

[oxarbitrage]

Hey, thank you for the contribution. Can you show us how were you able to see that block is not contextually valid: block height 12 is lower than the current finalized height 127 is now the error message ? I was wondering if we could add this to a test but maybe it will not worth it.

[teor2345]

@Mergifyio update

[mergify]

update

❌ Base branch update has failed

refusing to allow a GitHub App to create or update workflow .github/workflows/continous-delivery.yml without workflows permission
err-code: FAB54

[teor2345]

Hey, thank you for the contribution. Can you show us how were you able to see that block is not contextually valid: block height 12 is lower than the current finalized height 127 is now the error message ? I was wondering if we could add this to a test but maybe it will not worth it.

We don't usually test log messages, because they aren't part of Zebra's supported functionality. (We only use them in acceptance tests to see that the node has done the thing we actually want to test.)

[dimxy]

here is how this fix is used in my branch: https://github.com/dimxy/zebra/blob/0a5eaa4e9d59cdaa3a937d116ed0e6aa4b5a8b76/zebra-state/src/service/check/tests/komodo_nn.rs#L193:
it prints the ValidateContextError message in addition to CommitBlockError default message ("block is not contextually valid"):

commit_result1=block is not contextually valid: immature transparent coinbase spend: attempt to spend OutPoint { hash: transaction::Hash("cd5b8e29b0c893c57cc3c30d170f99a82e56157e3992374faad6d242ba0863ae"), index: 0 } at Height(139), but spends are invalid before Height(232), which is 100 blocks after it was created at Height(132)

Command to run: cargo test -p zebra-state komodo_nn::komodo_valid_fork_from_below_last_notarised_height

[teor2345]

here is how this fix is used in my branch: https://github.com/dimxy/zebra/blob/0a5eaa4e9d59cdaa3a937d116ed0e6aa4b5a8b76/zebra-state/src/service/check/tests/komodo_nn.rs#L193: it prints the ValidateContextError message in addition to CommitBlockError default message ("block is not contextually valid"):

commit_result1=block is not contextually valid: immature transparent coinbase spend: attempt to spend OutPoint { hash: transaction::Hash("cd5b8e29b0c893c57cc3c30d170f99a82e56157e3992374faad6d242ba0863ae"), index: 0 } at Height(139), but spends are invalid before Height(232), which is 100 blocks after it was created at Height(132)

Command to run: cargo test -p zebra-state komodo_nn::komodo_valid_fork_from_below_last_notarised_height

Are you trying to track down this bug?

• Fix duplicate transparent spends in getblocktemplate RPC responses #6195

[teor2345]

Note for the Zebra developers: we'll need to re-push this PR to our repository to get the Google Cloud tests to run, because the secrets are only available to developers with write access.

[dimxy]

here is how this fix is used in my branch: https://github.com/dimxy/zebra/blob/0a5eaa4e9d59cdaa3a937d116ed0e6aa4b5a8b76/zebra-state/src/service/check/tests/komodo_nn.rs#L193: it prints the ValidateContextError message in addition to CommitBlockError default message ("block is not contextually valid"):

commit_result1=block is not contextually valid: immature transparent coinbase spend: attempt to spend OutPoint { hash: transaction::Hash("cd5b8e29b0c893c57cc3c30d170f99a82e56157e3992374faad6d242ba0863ae"), index: 0 } at Height(139), but spends are invalid before Height(232), which is 100 blocks after it was created at Height(132)

Command to run: cargo test -p zebra-state komodo_nn::komodo_valid_fork_from_below_last_notarised_height

Are you trying to track down this bug?

• Fix invalid transparent spends in getblocktemplate RPC responses #6195

No, we are just playing with our project's params (coinbase maturity for testnet)

[upbqdn]

we'll need to re-push this PR to our repository

I re-pushed the branch from this PR to the Zebra repo here https://github.com/ZcashFoundation/zebra/tree/fix-commit-block-error-msg, but I don't know what to do next since I can't change the source branch of this PR.

[teor2345]

we'll need to re-push this PR to our repository

I re-pushed the branch from this PR to the Zebra repo here https://github.com/ZcashFoundation/zebra/tree/fix-commit-block-error-msg, but I don't know what to do next since I can't change the source branch of this PR.

I opened a PR at #6251, anyone can approve it.

Because the PR is from a branch in our repository, it has access to our GitHub secrets for CI. We want to fix this eventually in #4529.

[teor2345]

I'll need to close this PR to get Mergify to check the other PR, but we are going to merge it!

[mergify]

⚠️ The sha of the head commit of this PR conflicts with #6251. Mergify cannot evaluate rules on this PR. ⚠️