Page MenuHomePhabricator

refactor: Move functions to BlockManager methods
ClosedPublic

Authored by PiRK on May 20 2024, 09:47.

Details

Reviewers
Fabien
Group Reviewers
Restricted Project
Commits
rABC99f7dd2d8b85: refactor: Move functions to BlockManager methods
Summary

This is a commit in preparation for the next few commits. The functions
are moved to methods to avoid their re-declaration for the purpose of
passing in BlockManager options.

The functions that were now moved into the BlockManager should no longer
use the params as an argument, but instead use the member variable.

In the moved ReadBlockFromDisk and UndoReadFromDisk, change
the function signature to accept a reference to a CBlockIndex instead of
a raw pointer. The pointer is expected to be non-null, so reflect that
in the type.

To allow for the move of functions to BlockManager methods all call
sites require an instantiated BlockManager, or a callback to one.

This is a partial backport of core#27125 and core#25016
https://github.com/bitcoin/bitcoin/pull/27125/commits/f0bb1021f0d60f5f19176e67a66fcf7c325f88d1
https://github.com/bitcoin/bitcoin/pull/25016/commits/ed12c0a49d3c64d170aca9e66ef32a57d7933eeb
https://github.com/bitcoin/bitcoin/pull/25016/commits/86ce844d3b287012f27c7b0bad6d11c9bdd3120e

Depends on D16183

Test Plan

With Chronik and bitcoin-chainstate:
ninja all check-all

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

PiRK requested review of this revision.May 20 2024, 09:47

Tail of the build log:

test chronik/bitcoinsuite-core/src/bytes.rs - bytes::read_array (line 55) ... ok
test chronik/bitcoinsuite-core/src/bytes.rs - bytes::read_bytes (line 19) ... ok
test chronik/bitcoinsuite-core/src/hash.rs - hash::Hashed::digest (line 76) ... ok
test chronik/bitcoinsuite-core/src/hash.rs - hash::Hashed::from_be_slice (line 264) ... ok
test chronik/bitcoinsuite-core/src/hash.rs - hash::Hashed::from_le_hex (line 289) ... ok
test chronik/bitcoinsuite-core/src/script/opcode.rs - script::opcode::Opcode (line 12) ... ok
test chronik/bitcoinsuite-core/src/script/opcode.rs - script::opcode::Opcode::number (line 75) ... ok
test chronik/bitcoinsuite-core/src/hash.rs - hash::Hashed::from_le_slice (line 242) ... ok
test chronik/bitcoinsuite-core/src/hash.rs - hash::Hashed::to_be_bytes (line 150) ... ok
test chronik/bitcoinsuite-core/src/hash.rs - hash::Hashed::hex_be (line 190) ... ok
test chronik/bitcoinsuite-core/src/script/opcode.rs - script::opcode::opcode_number_to_name (line 86) ... ok
test chronik/bitcoinsuite-core/src/hash.rs - hash::Hashed::hex_le (line 168) ... ok
test chronik/bitcoinsuite-core/src/hash.rs - hash::Hashed::to_le_bytes (line 138) ... ok
test chronik/bitcoinsuite-core/src/hash.rs - hash::Hashed::to_le_vec (line 208) ... ok
test chronik/bitcoinsuite-core/src/hash.rs - hash::Hashed::to_be_vec (line 225) ... ok
test chronik/bitcoinsuite-core/src/script/pubkey.rs - script::pubkey::PubKey (line 13) ... ok
test chronik/bitcoinsuite-core/src/script/pubkey.rs - script::pubkey::PubKey::array (line 47) ... ok
test chronik/bitcoinsuite-core/src/script/pubkey.rs - script::pubkey::PubKey::hex (line 57) ... ok
test chronik/bitcoinsuite-core/src/script/pubkey.rs - script::pubkey::PubKey::as_slice (line 37) ... ok
test chronik/bitcoinsuite-core/src/script/pubkey_variant.rs - script::pubkey_variant::PubKeyVariant (line 14) ... ok
test chronik/bitcoinsuite-core/src/script/script.rs - script::script::Script::is_opreturn (line 159) ... ok
test chronik/bitcoinsuite-core/src/script/script.rs - script::script::Script::hex (line 148) ... ok
test chronik/bitcoinsuite-core/src/script/script.rs - script::script::Script::bytecode (line 126) ... ok
test chronik/bitcoinsuite-core/src/script/script.rs - script::script::Script::p2pk (line 76) ... ok
test chronik/bitcoinsuite-core/src/script/script.rs - script::script::Script::p2pk_uncompressed (line 98) ... ok
test chronik/bitcoinsuite-core/src/script/script.rs - script::script::Script::p2pkh (line 36) ... ok
test chronik/bitcoinsuite-core/src/script/script.rs - script::script::Script::p2sh (line 56) ... ok
test chronik/bitcoinsuite-core/src/script/script.rs - script::script::Script::iter_ops (line 174) ... ok
test chronik/bitcoinsuite-core/src/script/script.rs - script::script::Script::to_vec (line 137) ... ok
test chronik/bitcoinsuite-core/src/script/script.rs - script::script::Script::variant (line 223) ... ok
test chronik/bitcoinsuite-core/src/script/script_mut.rs - script::script_mut::ScriptMut::freeze (line 135) ... ok
test chronik/bitcoinsuite-core/src/script/script_mut.rs - script::script_mut::ScriptMut::put_bytecode (line 44) ... ok
test chronik/bitcoinsuite-core/src/script/script_mut.rs - script::script_mut::ScriptMut::put_opcodes (line 28) ... ok
test chronik/bitcoinsuite-core/src/script/script_mut.rs - script::script_mut::ScriptMut::put_slp_pushdata (line 116) ... ok
test chronik/bitcoinsuite-core/src/script/uncompressed_pubkey.rs - script::uncompressed_pubkey::UncompressedPubKey::array (line 50) ... ok
test chronik/bitcoinsuite-core/src/script/uncompressed_pubkey.rs - script::uncompressed_pubkey::UncompressedPubKey (line 13) ... ok
test chronik/bitcoinsuite-core/src/script/script_mut.rs - script::script_mut::ScriptMut::with_capacity (line 17) ... ok
test chronik/bitcoinsuite-core/src/script/script_mut.rs - script::script_mut::ScriptMut::put_pushdata (line 58) ... ok
test chronik/bitcoinsuite-core/src/script/uncompressed_pubkey.rs - script::uncompressed_pubkey::UncompressedPubKey::hex (line 60) ... ok
test chronik/bitcoinsuite-core/src/tx/tx.rs - tx::tx::Tx (line 34) - compile fail ... ok
test chronik/bitcoinsuite-core/src/script/uncompressed_pubkey.rs - script::uncompressed_pubkey::UncompressedPubKey::as_slice (line 40) ... ok
test chronik/bitcoinsuite-core/src/script/variant.rs - script::variant::ScriptVariant::from_type_and_payload (line 94) ... ok
test chronik/bitcoinsuite-core/src/tx/txid.rs - tx::txid::TxId::new (line 47) ... ok
test chronik/bitcoinsuite-core/src/script/variant.rs - script::variant::ScriptVariant::to_script (line 108) ... ok
test chronik/bitcoinsuite-core/src/tx/tx.rs - tx::tx::Tx (line 14) ... ok
test chronik/bitcoinsuite-core/src/tx/txid.rs - tx::txid::TxId::as_bytes (line 89) ... ok
test chronik/bitcoinsuite-core/src/tx/txid.rs - tx::txid::TxId::to_vec (line 104) ... ok
test chronik/bitcoinsuite-core/src/tx/txid.rs - tx::txid::TxId::to_bytes (line 74) ... ok
test chronik/bitcoinsuite-core/src/tx/txid.rs - tx::txid::TxId::from_tx (line 59) ... ok

test result: ok. 57 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 1.52s

   Doc-tests bitcoinsuite-slp

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

ninja: build stopped: cannot make progress due to previous errors.
Build build-chronik failed with exit code 1

Tail of the build log:

  Downloaded backtrace v0.3.71
  Downloaded indenter v0.3.3
  Downloaded http v1.1.0
    Blocking waiting for file lock on package cache
    Blocking waiting for file lock on build directory
   Compiling gimli v0.28.1
   Compiling adler v1.0.2
   Compiling memchr v2.7.2
   Compiling indenter v0.3.3
   Compiling once_cell v1.19.0
   Compiling rustc-demangle v0.1.23
   Compiling libc v0.2.153
   Compiling fnv v1.0.7
   Compiling bytes v1.6.0
   Compiling itoa v1.0.11
   Compiling abc-rust-lint v0.1.0 (/work/chronik/abc-rust-lint)
   Compiling miniz_oxide v0.7.2
   Compiling eyre v0.6.12
   Compiling http v1.1.0
   Compiling object v0.32.2
   Compiling addr2line v0.21.0
   Compiling backtrace v0.3.71
   Compiling stable-eyre v0.2.2
   Compiling abc-rust-error v0.1.0 (/work/chronik/abc-rust-error)
    Finished test [unoptimized + debuginfo] target(s) in 20.90s
     Running unittests src/lib.rs (abc-ci-builds/build-chronik-plugins/cargo/build/debug/deps/abc_rust_error-b84853c5b8f811a6)

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

     Running tests/test_error.rs (abc-ci-builds/build-chronik-plugins/cargo/build/debug/deps/test_error-1ab185c6894988ed)

running 1 test
test test_error ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

     Running unittests src/lib.rs (abc-ci-builds/build-chronik-plugins/cargo/build/debug/deps/abc_rust_lint-95bfaf7c76bca9d6)

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

   Doc-tests abc-rust-error

running 1 test
test chronik/abc-rust-error/src/http_status.rs - http_status::parse_error_status (line 10) ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.17s

   Doc-tests abc-rust-lint

running 1 test
test chronik/abc-rust-lint/src/lib.rs - lint (line 13) ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.11s

ninja: build stopped: cannot make progress due to previous errors.
Build build-chronik-plugins failed with exit code 1
Fabien requested changes to this revision.May 20 2024, 11:15
Fabien added a subscriber: Fabien.

Back to your queue

This revision now requires changes to proceed.May 20 2024, 11:15

rebase (check-crates should be fixed by D16189)

Fabien requested changes to this revision.May 21 2024, 08:40
Fabien added inline comments.
src/node/blockstorage.h
286 ↗(On Diff #47842)

why TODO ?

292 ↗(On Diff #47842)

Why can't it be a member function as well ?

This revision now requires changes to proceed.May 21 2024, 08:40
PiRK edited the summary of this revision. (Show Details)

move GetFirstStoredBlock as well (core#25016), remove leftover TODO

src/node/blockstorage.h
286 ↗(On Diff #47842)

that's a leftover. I did do it.

This revision is now accepted and ready to land.May 21 2024, 14:22