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

kernel: Handle fatal errors through return values #29642

Draft
wants to merge 27 commits into
base: master
Choose a base branch
from

Commits on Mar 26, 2024

  1. refactor: Drop util::Result operator=

    In cases where different error conditions need to be handled differently, it is
    useful for `util::`Result` objects to be able to hold failure values and error
    messages simultaneously. Also it is useful for `util::Result` objects to be
    able to hold multiple error message strings and warnings, instead of just
    single error strings.
    
    In both of these cases, which are supported in upcoming commits, having an
    `operator=` method is potentially dangerous if it clears existing error and
    warnings strings while setting result values, or confusing if it doesn't clear
    them, so the safest thing is to disable `operator=` and provide an explicit
    `Set()` method so callers have a way of setting result values without having to
    clear message strings.
    ryanofsky authored and TheCharlatan committed Mar 26, 2024
    Configuration menu
    Copy the full SHA
    440f674 View commit details
    Browse the repository at this point in the history
  2. refactor: Avoid copying util::Result values

    Copying util::Result values is less efficient than moving them because they
    allocate memory and contain strings. Also this is needed to avoid compile
    errors in the next commit which adds a std::unique_ptr member to util::Result
    which implicity disables copying.
    ryanofsky authored and TheCharlatan committed Mar 26, 2024
    Configuration menu
    Copy the full SHA
    882577b View commit details
    Browse the repository at this point in the history
  3. refactor: Add util::Result failure values

    Add util::Result support for returning more error information. For better error
    handling, adds the ability to return a value on failure, not just a value on
    success. This is a key missing feature that made the result class not useful
    for functions like LoadChainstate() which produce different errors that need to
    be handled differently.
    
    This change also makes some more minor improvements:
    
    - Smaller type size. util::Result<int> is 16 bytes, and util::Result<void> is 8
      bytes. Previously util::Result<int> and util::Result<void> were 72 bytes.
    
    - More documentation and tests.
    ryanofsky authored and TheCharlatan committed Mar 26, 2024
    Configuration menu
    Copy the full SHA
    93cee98 View commit details
    Browse the repository at this point in the history
  4. Configuration menu
    Copy the full SHA
    015b011 View commit details
    Browse the repository at this point in the history
  5. refactor: Add util::Result multiple error and warning messages

    Add util::Result support for returning warning messages and multiple errors,
    not just a single error string. This provides a way for functions to report
    errors and warnings in a standard way, and simplifies interfaces.
    
    The functionality is unit tested here, and put to use in followup PR
    bitcoin#25722
    ryanofsky authored and TheCharlatan committed Mar 26, 2024
    Configuration menu
    Copy the full SHA
    c7d4f43 View commit details
    Browse the repository at this point in the history
  6. test: add static test for util::Result memory usage

    Suggested by Martin Leitner-Ankerl <martin.ankerl@gmail.com>
    bitcoin#25722 (comment)
    
    Co-authored-by: Martin Leitner-Ankerl <martin.ankerl@gmail.com>
    2 people authored and TheCharlatan committed Mar 26, 2024
    Configuration menu
    Copy the full SHA
    5e38b42 View commit details
    Browse the repository at this point in the history
  7. Add bitcoin-fatal-error clang-tidy check

    The check warns about incorrect usage of util::Result<T, FatalError>.
    The check enforces the following rules:
    
    1. If a function calls another function with a util::Result<T,
    FatalCondition> return type, its return type has to be util::Result<T,
    FatalCondition> too, or it has to handle the value returned by the
    function with one of "CheckFatal", "HandleFatalError",
    "UnwrapFatalError", "CheckFatalFailure".
    
    2. In functions returning a util::Result<T, FatalCondition> a call to a
    function returning a util::Result<T, FatalCondition> needs to propagate
    the value by either:
     a) Returning it immediately
     b) Assigning it immediately to an existing result with .MoveMessages()
        or .Set()
     c) Eventually passing it as an arugment to a .MoveMessages() call
    TheCharlatan committed Mar 26, 2024
    Configuration menu
    Copy the full SHA
    e5936fe View commit details
    Browse the repository at this point in the history
  8. kernel: Handle ImportBlocks fatal errors with Result<T, FatalError>

    This introduces a new FatalError enum class whose semantics when used as
    a failure type in a util::Result<T, FatalError> are controlled by the
    previously introduced bitcoin-fatal-error clang-tidy plugin.
    
    The kernel's `UnwrapFatalError` method is meant to be used in unit and
    fuzz tests, while production code should use `CheckFatal` to handle
    fatal errors.
    TheCharlatan committed Mar 26, 2024
    Configuration menu
    Copy the full SHA
    b16787d View commit details
    Browse the repository at this point in the history
  9. Configuration menu
    Copy the full SHA
    e641470 View commit details
    Browse the repository at this point in the history
  10. Configuration menu
    Copy the full SHA
    cf55203 View commit details
    Browse the repository at this point in the history
  11. Configuration menu
    Copy the full SHA
    1850f3b View commit details
    Browse the repository at this point in the history
  12. Configuration menu
    Copy the full SHA
    93b9c3b View commit details
    Browse the repository at this point in the history
  13. kernel: Handle ValidatedSnapshotCleanup fatal errors with Result<T, F…

    …atalError>
    
    This slightly refactors the method to return the FatalError instead of
    throwing.
    TheCharlatan committed Mar 26, 2024
    Configuration menu
    Copy the full SHA
    4bfa9da View commit details
    Browse the repository at this point in the history
  14. Configuration menu
    Copy the full SHA
    4e2de7b View commit details
    Browse the repository at this point in the history
  15. kernel: Handle AcceptBlock fatal errors with Result<T, FatalError>

    To propagate the FatalError, also add the type to
    InvalidateCoinsDBOnDisk and AcceptBlock.
    
    The net_processing module now has to handle a FatalError, so introduce
    shutdown and exit_status member variables for to be able to abort.
    TheCharlatan committed Mar 26, 2024
    Configuration menu
    Copy the full SHA
    0758b72 View commit details
    Browse the repository at this point in the history
  16. kernel: Handle ActivateBestChainStep fatal errors with Result<T, Fata…

    …lError>
    
    To propagate the FatalError, also add the type to PreciousBlock and
    ActivateBestChain.
    TheCharlatan committed Mar 26, 2024
    Configuration menu
    Copy the full SHA
    6eceef8 View commit details
    Browse the repository at this point in the history
  17. Configuration menu
    Copy the full SHA
    11a0d3f View commit details
    Browse the repository at this point in the history
  18. kernel: Handle FlushStateToDisk fatal errors with Result<T, FatalError>

    To propagate the FatalError, also add the type to PruneBlockFilesManual,
    AcceptToMemoryPool, ProcessNewPackage, ResizeCoinsCaches,
    ForceFlushStateToDisk, PruneAndFlush, DisconnectTip, InvalidateBlock,
    MaybeUpdateMempoolForReorg, ProcessTransaction, MaybeRebalanceCaches,
    LoadMempool
    TheCharlatan committed Mar 26, 2024
    Configuration menu
    Copy the full SHA
    42943e5 View commit details
    Browse the repository at this point in the history
  19. kernel: Handle ConnectBlock fatal errors with Result<T, FatalError>

    To propagate the FatalError, also add the type to VerifyDB and
    TestBlockValidity.
    
    The miner module now has to handle a FatalError, so introduce shutdown
    and exit_status member variables for it to be able to abort.
    TheCharlatan committed Mar 26, 2024
    Configuration menu
    Copy the full SHA
    a555387 View commit details
    Browse the repository at this point in the history
  20. kernel: Handle SaveBlockToDisk fatal errors with Result<T, FatalError>

    To propagate the FatalError, also add the type to LoadGenesisBlock.
    TheCharlatan committed Mar 26, 2024
    Configuration menu
    Copy the full SHA
    16dee3c View commit details
    Browse the repository at this point in the history
  21. Configuration menu
    Copy the full SHA
    f9b1773 View commit details
    Browse the repository at this point in the history
  22. kernel: Handle FlushBlockFile fatal errors with Result<T, FatalError>

    To propagate the FatalError, also add the type to
    FlushChainstateBlockFile.
    TheCharlatan committed Mar 26, 2024
    Configuration menu
    Copy the full SHA
    57b7430 View commit details
    Browse the repository at this point in the history
  23. Configuration menu
    Copy the full SHA
    738e850 View commit details
    Browse the repository at this point in the history
  24. Configuration menu
    Copy the full SHA
    0273acf View commit details
    Browse the repository at this point in the history
  25. Configuration menu
    Copy the full SHA
    76f1420 View commit details
    Browse the repository at this point in the history
  26. kernel: Handle LoadBlockIndex fatal errors with Result<T, FatalError>

    To propagate the FatalError, also add the type to LoadBlockIndexDB.
    TheCharlatan committed Mar 26, 2024
    Configuration menu
    Copy the full SHA
    e2c336b View commit details
    Browse the repository at this point in the history
  27. Remove flushError, fatalError from kernel notifications

    Also add using declarations where now possible.
    TheCharlatan committed Mar 26, 2024
    Configuration menu
    Copy the full SHA
    bd0a9bb View commit details
    Browse the repository at this point in the history