refactor: have CCoins* data managed under CChainState #16443

pull jamesob wants to merge 3 commits into bitcoin:master from jamesob:2019-07-au-coins-under-chainstate changing 17 files +247 −100
  1. jamesob commented at 7:27 PM on July 23, 2019: member

    This is part of the assumeutxo project:

    Parent PR: #15606 Issue: #15605 Specification: https://github.com/jamesob/assumeutxo-docs/tree/2019-04-proposal/proposal

    This change encapsulates UTXO set data within CChainState instances, removing global data pcoinsTip and pcoinsviewdb. This is necessary if we want to maintain multiple chainstates with their own rendering of the UTXO set.

    We introduce a class CoinsViews which consolidates the construction of a CCoins* hierarchy.

    This commit could be broken into smaller pieces, but it would require more ephemeral diffs to, e.g., temporarily change CCoinsViewDB's constructor invocations.

  2. fanquake added the label Refactoring on Jul 23, 2019
  3. in src/validation.h:23 in e56af43d8d outdated 
      18 | @@ -19,6 +19,8 @@
  19 |  #include <script/script_error.h>
  20 |  #include <sync.h>
  21 |  #include <txmempool.h> // For CTxMemPool::cs
  22 | +#include <txdb.h>
  23 | +#include <util/system.h>
    MarcoFalke commented at 7:48 PM on July 23, 2019:

    I'd prefer to make the call to GetDataDir in the cpp file.

  4. in src/validation.h:665 in e56af43d8d outdated 
     717 | @@ -659,11 +718,7 @@ CChain& ChainActive();
 718 |  /** @returns the global block index map. */
 719 |  BlockMap& BlockIndex();
 720 |  
 721 | -/** Global variable that points to the coins database (protected by cs_main) */
 722 | -extern std::unique_ptr<CCoinsViewDB> pcoinsdbview;
 723 | -
 724 | -/** Global variable that points to the active CCoinsView (protected by cs_main) */
    MarcoFalke commented at 7:48 PM on July 23, 2019:

    Why are you removing the comment that this data is protected by cs_main?

    ryanofsky commented at 6:14 PM on July 24, 2019:

    In commit "refactor: pcoinsTip -> CChainState::CoinsTip()" (9edf6537f13b61952f77ab62c2221c2b18d9561a)

    Why are you removing the comment that this data is protected by cs_main?

    Seems resolved now (comment is just moved).

    jamesob commented at 8:08 PM on July 24, 2019:

    Good point - since it was easy to do, I've tacked on a commit that adds lock annotations to the equivalent CoinsViews members.

  5. in src/validation.cpp:3876 in e56af43d8d outdated 
    3872 | @@ -3853,12 +3873,13 @@ bool static LoadBlockIndexDB(const CChainParams& chainparams) EXCLUSIVE_LOCKS_RE
3873 |  bool LoadChainTip(const CChainParams& chainparams)
3874 |  {
3875 |      AssertLockHeld(cs_main);
3876 | -    assert(!pcoinsTip->GetBestBlock().IsNull()); // Never called when the coins view is empty
3877 | +    CCoinsViewCache* coins_cache = &::ChainstateActive().CoinsTip();
    MarcoFalke commented at 7:50 PM on July 23, 2019: 
        const CCoinsViewCache& coins_cache = ::ChainstateActive().CoinsTip();
  6. in src/validation.cpp:88 in e56af43d8d outdated 
      85 | +std::unique_ptr<CChainState> g_chainstate;
  86 |  
  87 | -CChainState& ChainstateActive() { return g_chainstate; }
  88 | +CChainState& ChainstateActive() {
  89 | +    assert(g_chainstate);
  90 | +    return *g_chainstate.get();
    MarcoFalke commented at 7:52 PM on July 23, 2019: 
        return *g_chainstate;
  7. in src/net_processing.cpp:1294 in e56af43d8d outdated 
    1290 | @@ -1291,11 +1291,12 @@ bool static AlreadyHave(const CInv& inv) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
1291 |                  LOCK(g_cs_orphans);
1292 |                  if (mapOrphanTransactions.count(inv.hash)) return true;
1293 |              }
1294 | +            CCoinsViewCache& coins_cache = ::ChainstateActive().CoinsTip();
    MarcoFalke commented at 7:54 PM on July 23, 2019: 
                const CCoinsViewCache& coins_cache = ::ChainstateActive().CoinsTip();
  8. MarcoFalke commented at 7:59 PM on July 23, 2019: member

    Would it make sense to split this into dumb refactoring and then hard-to-review changes, like the on in init.cpp?

    An early commit (scripted-diff or not) could add ChainstateActive::CoinsTip, a dumb alias to return pcoinsTip; and replace the usages. The later commit could then make the other changes in this pull.

  9. DrahtBot commented at 10:30 PM on July 23, 2019: member

    <!--e57a25ab6845829454e8d69fc972939a-->

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #16411 (Signet support by kallewoof)
    • #16400 ([refactor] Rewrite AcceptToMemoryPoolWorker() using smaller parts by sdaftuar)
    • #16324 (Get cs_main out of the critical path in ProcessMessages by TheBlueMatt)
    • #16323 (Call ProcessNewBlock() asynchronously by TheBlueMatt)
    • #15997 (refactor: GuessVerificationProgress requires cs_main lock by promag)
    • #13868 (Remove unused fScriptChecks parameter from CheckInputs by Empact)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

  10. fanquake added this to the "In progress" column in a project
  11. jamesob force-pushed on Jul 24, 2019
  12. in src/validation.cpp:522 in 9edf6537f1 outdated 
     512 | @@ -513,12 +513,13 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
 513 |          CCoinsViewCache view(&dummy);
 514 |  
 515 |          LockPoints lp;
 516 | -        CCoinsViewMemPool viewMemPool(pcoinsTip.get(), pool);
 517 | +        CCoinsViewCache* coins_cache = &::ChainstateActive().CoinsTip();
    ryanofsky commented at 6:11 PM on July 24, 2019:

    In commit "refactor: pcoinsTip -> CChainState::CoinsTip()" (9edf6537f13b61952f77ab62c2221c2b18d9561a)

    Here and in rpc/blockchain.cpp, if you're going to introduce a new variable, it seems better to make it a reference than a raw pointer. (IMO at least it is best to reserve c++ pointers for nullable cases to refer to objects that may or may not exist, and c++ references to refer to objects that are assumed to exist).

  13. DrahtBot added the label Needs rebase on Jul 24, 2019
  14. in src/validation.cpp:1053 in e5d83bf15b outdated 
    1042 | +    std::string ldb_name,
1043 | +    size_t cache_size_bytes,
1044 | +    bool in_memory,
1045 | +    bool should_wipe)
1046 | +{
1047 | +    m_dbview.reset(new CCoinsViewDB(
    ryanofsky commented at 6:21 PM on July 24, 2019:

    In commit "refactor: add CoinsViews class" (e5d83bf15bbf1f9359dcb6146dc5f12ae61d8720)

    This doesn't seem to compile. CCoinsViewDB at this point takes 3 arguments not 4. Also could avoid raw pointer management and new call with member initialization and MakeUnique:

    : m_dbview(MakeUnique<CCoinsViewDB>(cache_size_bytes, in_memory, should_wipe))

    Member initialization is better because it ensure objects are destroyed in reverse order they are constructed, and raw pointers and news are just generally suspicious and should be avoided when you can make ownership explicit.

    jamesob commented at 8:15 PM on July 24, 2019:

    Ah, oops - you're right. Shouldn't have tried to split those commits out.

  15. in src/rpc/blockchain.cpp:1066 in ceca2c92b7 outdated 
    1061 | @@ -1062,6 +1062,8 @@ static UniValue gettxoutsetinfo(const JSONRPCRequest& request)
1062 |  
1063 |      CCoinsStats stats;
1064 |      ::ChainstateActive().ForceFlushStateToDisk();
1065 | +
1066 | +    LOCK(::cs_main);
    ryanofsky commented at 7:06 PM on July 24, 2019:

    In commit "Cover UTXO set access with lock annotations" (ceca2c92b79a8a2c5ae37d03e99de80ec55895c3)

    Commit message is a bit misleading imo, because this is adding a new lock, not just new annotations.

    But more importantly, GetUTXOStats is acquiring and releasing cs_main internally, so this change seems like it is complicating locking and also locking too much. Maybe a change like the following would make sense?

    -    LOCK(::cs_main);
-    if (GetUTXOStats(&::ChainstateActive().CoinsDB(), stats)) {
+    CCoinsView* coins_view = WITH_LOCK(cs_main, return &ChainstateActive().CoinsDB());
+    if (GetUTXOStats(coins_view, stats)) {
    jamesob commented at 8:25 PM on July 24, 2019:

    Ah that's a good point - I forgot that GetUTXOStats only briefly acquires cs_main to get the height for a certain blockhash; it (counterintuitively) doesn't need to hold cs_main to iterate through the coins db because leveldb's implicit use of snapshots with cursors.

    I think the WITH_LOCK usage above obeys the letter of the law but not the spirit since technically the coinsdb could be modified in between that line and the GetUTXOStats call, but in this context I don't think it matters. Thanks for the suggestion!

  16. in src/validation.h:750 in eed4e62ad6 outdated 
     713 | @@ -686,8 +714,7 @@ CChain& ChainActive();
 714 |  /** @returns the global block index map. */
 715 |  BlockMap& BlockIndex();
 716 |  
 717 | -/** Global variable that points to the coins database (protected by cs_main) */
 718 | -extern std::unique_ptr<CCoinsViewDB> pcoinsdbview;
 719 | +extern std::unique_ptr<CChainState> g_chainstate;
    ryanofsky commented at 7:30 PM on July 24, 2019:

    In commit "refactor: have CCoins* data managed under CChainState" (eed4e62ad6b162ff0d3e2b87eb8762a689678bca)

    Can you add a comment here about when it's appropriate to access g_chainstate variable directly, as opposed to calling ChainstateActive()? I see code in init.cpp is using both, but mostly the former, which is different than what I'd expect. I'd expect init code to just create a CChainState object and assign it it to g_chainstate once rather than accessing the g_chainstate global repeatedly.

  17. in src/validation.cpp:1069 in eed4e62ad6 outdated 
    1062 | +    std::string leveldb_name
1063 | +    // NOTE: for now m_blockman is set to a global, but this will be changed
1064 | +    // in a future commit.
1065 | +    ) : m_blockman(g_blockman)
1066 | +{
1067 | +    m_coins_views.reset(new CoinsViews(
    ryanofsky commented at 7:33 PM on July 24, 2019:

    In commit "refactor: have CCoins* data managed under CChainState" (eed4e62ad6b162ff0d3e2b87eb8762a689678bca)

    Suggest x = MakeUnique<X>() instead of x.reset(new X()) to avoid raw pointers and new in C++, which are suspicious.

  18. in src/init.cpp:1465 in eed4e62ad6 outdated 
    1459 | @@ -1464,10 +1460,18 @@ bool AppInitMain(InitInterfaces& interfaces)
1460 |              bool is_coinsview_empty;
1461 |              try {
1462 |                  LOCK(cs_main);
1463 | +                g_chainstate.reset(new CChainState(
    ryanofsky commented at 7:33 PM on July 24, 2019:

    In commit "refactor: have CCoins* data managed under CChainState" (eed4e62ad6b162ff0d3e2b87eb8762a689678bca)

    Suggest x = MakeUnique<X>() instead of x.reset(new X()) to avoid raw pointers and new in C++, which are suspicious.

  19. in src/test/setup_common.cpp:88 in eed4e62ad6 outdated 
      81 | @@ -82,8 +82,8 @@ TestingSetup::TestingSetup(const std::string& chainName) : BasicTestingSetup(cha
  82 |  
  83 |      mempool.setSanityCheck(1.0);
  84 |      pblocktree.reset(new CBlockTreeDB(1 << 20, true));
  85 | -    pcoinsdbview.reset(new CCoinsViewDB(1 << 23, true));
  86 | -    pcoinsTip.reset(new CCoinsViewCache(pcoinsdbview.get()));
  87 | +    g_chainstate.reset(new CChainState(
    ryanofsky commented at 7:33 PM on July 24, 2019:

    In commit "refactor: have CCoins* data managed under CChainState" (eed4e62ad6b162ff0d3e2b87eb8762a689678bca)

    Suggest x = MakeUnique<X>() instead of x.reset(new X()) to avoid raw pointers and new in C++, which are suspicious.

  20. in src/init.cpp:1523 in eed4e62ad6 outdated 
    1521 | @@ -1518,33 +1522,23 @@ bool AppInitMain(InitInterfaces& interfaces)
1522 |                  // At this point we're either in reindex or we've loaded a useful
1523 |                  // block tree into BlockIndex()!
1524 |  
1525 | -                pcoinsdbview.reset(new CCoinsViewDB(nCoinDBCache, false, fReset || fReindexChainState));
    ryanofsky commented at 7:38 PM on July 24, 2019:

    In commit "refactor: have CCoins* data managed under CChainState" (eed4e62ad6b162ff0d3e2b87eb8762a689678bca)

    Could you specify in the commit message whether this is changing initialization order in any significant way? It seems like utxo database is being loaded a lot earlier now.

  21. in src/txdb.h:57 in eed4e62ad6 outdated 
      53 | +     * @param[in] ldb_path    Location in the filesystem where leveldb data will be stored.
  54 | +     */
  55 | +    explicit CCoinsViewDB(
  56 | +        fs::path ldb_path,
  57 | +        size_t nCacheSize,
  58 | +        bool fMemory = false,
    ryanofsky commented at 7:41 PM on July 24, 2019:

    In commit "refactor: have CCoins* data managed under CChainState" (eed4e62ad6b162ff0d3e2b87eb8762a689678bca)

    Can we get rid of these default values? It seems like they are not actually used anymore, and default booleans params in general tend to make code trickier to read and mask bugs.

  22. jamesob force-pushed on Jul 24, 2019
  23. ryanofsky approved
  24. ryanofsky commented at 7:47 PM on July 24, 2019: member

    utACK ceca2c92b79a8a2c5ae37d03e99de80ec55895c3. This looks good and is very straightforward. Feel free to ignore all my suggestions below! (They are not important and possibly not coherent.)

  25. DrahtBot removed the label Needs rebase on Jul 24, 2019
  26. jamesob force-pushed on Jul 24, 2019
  27. jamesob force-pushed on Jul 24, 2019
  28. jamesob commented at 9:52 PM on July 24, 2019: member

    Thanks for the feedback, @ryanofsky. I think I've incorporated all of it.

  29. in src/validation.h:523 in 09e6959ccb outdated 
     516 | +
 517 | +public:
 518 | +    std::unique_ptr<CCoinsViewDB> m_dbview;
 519 | +    std::unique_ptr<CCoinsViewErrorCatcher> m_catcherview;
 520 | +    std::unique_ptr<CCoinsViewCache> m_cacheview;
 521 | +
    ariard commented at 12:33 AM on July 25, 2019:

    nit: You could document this constructor, at least referring to doc fields in CDBWrapper declaration

    super-nit: Also at first read, I found CoinsViews name confusing because we already have CoinsView, we could have CoinsViewSet

  30. in src/validation.h:509 in 5fa4763f53 outdated 
     504 | @@ -505,6 +505,9 @@ class BlockManager {
 505 |          CBlockIndex** ppindex) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
 506 |  };
 507 |  
 508 | +/** Global variable that points to the active CCoinsView (protected by cs_main) */
 509 | +extern std::unique_ptr<CCoinsViewCache> pcoinsTip;
    ariard commented at 1:23 AM on July 25, 2019:

    Did you remove it in last commit ? I still see it at d0e1e37

    ariard commented at 1:27 AM on July 25, 2019:

    Also comment in ATMP refers to it

    jamesob commented at 1:59 PM on July 25, 2019:

    Good catch! Forgot to remove this after I split out the CoinsTip() rename commit.

  31. in src/validation.h:608 in 09e6959ccb outdated 
     582 | +    CChainState(
 583 | +        /* parameters forwarded to CoinsViews */
 584 | +        size_t cache_size_bytes,
 585 | +        bool in_memory,
 586 | +        bool should_wipe,
 587 | +        std::string leveldb_name = "chainstate");
    ariard commented at 1:43 AM on July 25, 2019:

    Hmmm is there any risk than having a default leveldb_name cause database collisions for different CChainState at some point ?

    jamesob commented at 2:14 PM on July 25, 2019:

    This seems like something that would be caught pretty easily in tests if the confusion ever did happen, and I'm not sure if repeating "chainstate" in a few different places inline is any better. That said I'm happy to make this change if others agree it seems worthwhile.

  32. in src/validation.h:518 in 09e6959ccb outdated 
     513 | + * to facilitate access to the UTXO set.
 514 | + */
 515 | +class CoinsViews {
 516 | +
 517 | +public:
 518 | +    std::unique_ptr<CCoinsViewDB> m_dbview;
    ariard commented at 1:45 AM on July 25, 2019:

    nit : I know these fields weren't documented as global, but maybe add comment explaining the aforementioned hierarchy between them

  33. ariard approved
  34. ariard commented at 1:51 AM on July 25, 2019: member

    ACK d0e1e37 minus pcoinsTip removal.

    Also I checked the other access to CoinsViews members via CoinsTip like in src/rpc/blockchain.cpp or src/rpc/rawtransaction.cpp. I've found every to be annotated or with a lock before, but was wandering if we shouldn't rely on EXCLUSIVE_LOCK_REQUIRED if possible and not AssertLockHeld. What's the exact diff between them ?

  35. sipa commented at 1:55 AM on July 25, 2019: member

    EXCLUSIVE_LOCK_REQUIRED is checked at compile time, but only when the compiler has certain flags enabled (only on Clang, I believe).

    AssertLockHeld is checked at runtime when lock debugging is compiled in (available for all compilers, but only enabled in debug builds).

  36. ariard commented at 2:01 AM on July 25, 2019: member

    Yes EXCLUSIVE_LOCK_REQUIRED is defined in threadsafety.h and is part of Clang static analysis on Mac OS X. But reviewing last commit, e.g in ActivateBestChainStep we only use AssertLockHeld but in fact we should use Clang attribute (or maybe both). Is there a consistent policy on which one to favour ?

  37. jamesob force-pushed on Jul 25, 2019
  38. jamesob commented at 3:30 PM on July 25, 2019: member

    Thanks for the look, @ariard. Feedback pushed.

  39. ryanofsky approved
  40. ryanofsky commented at 4:27 AM on July 26, 2019: member

    utACK c860b44ce33f424de7f0fe5f96efa65c58bdb531. There have just been a lot of small changes since last review: rebase for bilingual_str, many new code comments and commit comments, more MakeUniques and c++ references replacing raw pointers, avoiding recursive cs_main lock in gettxoutsetinfo.

    re: #16443 (comment)

    Is there a consistent policy on which one to favour ?

    It's good to use EXCLUSIVE_LOCK_REQUIRED wherever possible. AssertLockHeld is older and not used as much recently, but still useful for compilers other than clang and cases where clang annotations don't work (sometimes when there are private mutexes, locks in callbacks and virtual methods).

  42. MarcoFalke commented at 12:15 PM on July 26, 2019: member

    In commit 53c8d59d18 refactor: have CCoins* data managed under CChainState

       - The initialization order in init.cpp is changed slightly: the coinsdb (leveldb)
      creation is moved to be earlier than block index and genesis block loading,
      though this shouldn't have any noticeable effect.

    Could you elaborate a bit more why it is save to flush the chainstate when shutdown is requested during the init sequence? As I read the code, previously we wouldn't flush and now we flush an empty db (potentially not having the best block hash set, because genesis is not loaded?).

  43. in src/validation.cpp:2214 in c860b44ce3 outdated 
    2204 | @@ -2177,9 +2205,10 @@ void static UpdateTip(const CBlockIndex *pindexNew, const CChainParams& chainPar
2205 |    * disconnectpool (note that the caller is responsible for mempool consistency
2206 |    * in any case).
2207 |    */
2208 | -bool CChainState::DisconnectTip(CValidationState& state, const CChainParams& chainparams, DisconnectedBlockTransactions *disconnectpool)
2209 | +bool CChainState::DisconnectTip(CValidationState& state, const CChainParams& chainparams, DisconnectedBlockTransactions* disconnectpool)
2210 | +    EXCLUSIVE_LOCKS_REQUIRED(::cs_main)
2211 |  {
2212 | -    CBlockIndex *pindexDelete = m_chain.Tip();
2213 | +    CBlockIndex* pindexDelete = m_chain.Tip();
    MarcoFalke commented at 12:35 PM on July 26, 2019:

    Why are you removing the mempool lock annotation? Aren't the lock annotations in the header correct?

    jamesob commented at 3:07 PM on July 26, 2019:

    Ah you're right - dumb mistake on my part.

  44. in src/validation.cpp:2330 in c860b44ce3 outdated 
    2325 | @@ -2297,7 +2326,8 @@ class ConnectTrace {
2326 |   *
2327 |   * The block is added to connectTrace if connection succeeds.
2328 |   */
2329 | -bool CChainState::ConnectTip(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexNew, const std::shared_ptr<const CBlock>& pblock, ConnectTrace& connectTrace, DisconnectedBlockTransactions &disconnectpool)
2330 | +bool CChainState::ConnectTip(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexNew, const std::shared_ptr<const CBlock>& pblock, ConnectTrace& connectTrace, DisconnectedBlockTransactions& disconnectpool)
2331 | +    EXCLUSIVE_LOCKS_REQUIRED(::cs_main)
    MarcoFalke commented at 12:35 PM on July 26, 2019:

    Same

  45. MarcoFalke commented at 12:35 PM on July 26, 2019: member

    ACK c860b44ce3, code looks good overall. Running into segfaults when testing

    <details><summary>Show signature and timestamp</summary>

    Signature:

    -----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

ACK c860b44ce3, code looks good overall. Running into segfaults when testing
-----BEGIN PGP SIGNATURE-----

iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
pUi8YAwAnzGBHNruwZp6wvxhnfnew86jtl74PDidtnPwvGX9JiMOeY07UF70JOnO
OwTXN9IV7ZSAQklT0qik7QWAodG0h5TARUsmhnBfITrBTka81h+6JIN/ZTLq+abC
katI8VltU56K8HeXEcYTXuPWN3pjJAFlVphLPgvtxp5EwsFvykK6iebSKAKFfe5w
dCxSTuL/G/T+PAQNMXxAGS5MiheRFHu7I70ES9ddRuH4qGkVVutnjY9skajcX2+a
w9m1s/fFovci/bOaYCfQqfYlWXLHnOS+ewl9kGOPC3re6l3byY4+Lr19cmrDlNE+
qp3igws7FnoOJOjETY5D5sBvd+AikZcBk9uvVhWubPpJ5uLHQJactj/UigJtfGsJ
rKCpwX7oN30ZeSRf3vjCDUqO6HKKHih9dsPhQ8ikjswfqqTuMU0oPoNL+9PSaZi7
k9X0qcTpsC0M+Da+K2egehus507NFwpLLnMAWftLhzQoIxdt4/YHNEf0QT7P2L5F
5zZwImCS
=zVl4
-----END PGP SIGNATURE-----

    Timestamp of file with hash ccd43c0d933c3fca4055f6f6006fddbebd4ec81c67d0810961187dc2dab9838d -

    </details>

  46. MarcoFalke commented at 12:35 PM on July 26, 2019: member

    I am seeing segmentation faults when shutting down during init:

    ==4778== Thread 9 bitcoin-shutoff:
==4778== Invalid read of size 4
==4778==    at 0x4953BC: FlushBlockFile(bool) (validation.cpp:1548)
==4778==    by 0x496B76: CChainState::FlushStateToDisk(CChainParams const&, CValidationState&, FlushStateMode, int) (validation.cpp:2061)
==4778==    by 0x497AB5: CChainState::ForceFlushStateToDisk() (validation.cpp:2115)
==4778==    by 0x32AE19: Shutdown(InitInterfaces&) (init.cpp:239)
==4778==    by 0x223F6F: BitcoinCore::shutdown() (bitcoin.cpp:161)
==4778==    by 0x58F4BF9: QObject::event(QEvent*) (in /usr/lib64/libQt5Core.so.5.12.4)
==4778==    by 0x4BFCAF5: QApplicationPrivate::notify_helper(QObject*, QEvent*) (in /usr/lib64/libQt5Widgets.so.5.12.4)
==4778==    by 0x4C05E7F: QApplication::notify(QObject*, QEvent*) (in /usr/lib64/libQt5Widgets.so.5.12.4)
==4778==    by 0x58C9AE7: QCoreApplication::notifyInternal2(QObject*, QEvent*) (in /usr/lib64/libQt5Core.so.5.12.4)
==4778==    by 0x58CCA92: QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) (in /usr/lib64/libQt5Core.so.5.12.4)
==4778==    by 0x591EE46: ??? (in /usr/lib64/libQt5Core.so.5.12.4)
==4778==    by 0x6F2EEDC: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.6000.5)
==4778==  Address 0x4 is not stack'd, malloc'd or (recently) free'd
==4778== 
==4778== 
==4778== Process terminating with default action of signal 11 (SIGSEGV): dumping core
==4778==  Access not within mapped region at address 0x4
==4778==    at 0x4953BC: FlushBlockFile(bool) (validation.cpp:1548)
==4778==    by 0x496B76: CChainState::FlushStateToDisk(CChainParams const&, CValidationState&, FlushStateMode, int) (validation.cpp:2061)
==4778==    by 0x497AB5: CChainState::ForceFlushStateToDisk() (validation.cpp:2115)
==4778==    by 0x32AE19: Shutdown(InitInterfaces&) (init.cpp:239)
==4778==    by 0x223F6F: BitcoinCore::shutdown() (bitcoin.cpp:161)
==4778==    by 0x58F4BF9: QObject::event(QEvent*) (in /usr/lib64/libQt5Core.so.5.12.4)
==4778==    by 0x4BFCAF5: QApplicationPrivate::notify_helper(QObject*, QEvent*) (in /usr/lib64/libQt5Widgets.so.5.12.4)
==4778==    by 0x4C05E7F: QApplication::notify(QObject*, QEvent*) (in /usr/lib64/libQt5Widgets.so.5.12.4)
==4778==    by 0x58C9AE7: QCoreApplication::notifyInternal2(QObject*, QEvent*) (in /usr/lib64/libQt5Core.so.5.12.4)
==4778==    by 0x58CCA92: QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) (in /usr/lib64/libQt5Core.so.5.12.4)
==4778==    by 0x591EE46: ??? (in /usr/lib64/libQt5Core.so.5.12.4)
==4778==    by 0x6F2EEDC: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.6000.5)
  47. jamesob force-pushed on Jul 26, 2019
  48. jamesob commented at 4:10 PM on July 26, 2019: member

    Good catches, @MarcoFalke! I've pushed fixes, including https://github.com/bitcoin/bitcoin/pull/16443/commits/e4b3247d4a67f248ff13bf43c378337ee993555b to solve the init bug you found. There are a few ways of addressing that one but I think an early exit from FlushStateToDisk() is the cleanest since it doesn't involve partial construction of CoinsViews or introducing more have-I-initialized-yet state, but let me know if you think there's a better way.

  49. jamesob force-pushed on Jul 26, 2019
  50. ryanofsky commented at 5:41 PM on July 26, 2019: member

    In commit 53c8d59 refactor: have CCoins* data managed under CChainState

       - The initialization order in init.cpp is changed slightly: the coinsdb (leveldb)
      creation is moved to be earlier than block index and genesis block loading,
      though this shouldn't have any noticeable effect.

    Could you elaborate a bit more why it is save to flush the chainstate when shutdown is requested during the init sequence?

    I'm also curious about the answer to this. Would be good to say what's motivating the move in the comment. I guess I also had a similar question previously: #15606 (review)

  51. jamesob force-pushed on Jul 26, 2019
  52. jamesob commented at 7:53 PM on July 26, 2019: member

    @ryanofsky @MarcoFalke good points. I've abandoned trying to reorder the init.cpp stuff and now have separated the construction of CChainState from CoinsViews to preserve the existing order.

  53. ryanofsky approved
  54. ryanofsky commented at 7:35 PM on July 30, 2019: member

    utACK 4e307e5f5d676d01a94e1b645dcb0ee8574faf77. Only changes since last review are adding and using little InitCoinsViews / HasCoinsViews helpers so the init order doesn't have to change, and reverting the new FlushStateToDisk check which is no longer needed because the init order is not changing. Also, Marco's name is now added to the credits, but I can live with that.

    -Thanks to Russell Yanofsky for helpful feedback.
+Thanks to Russell Yanofsky and Marco Falke for helpful feedback.
  55. in src/validation.cpp:318 in 4e307e5f5d outdated 
     313 | @@ -314,7 +314,8 @@ bool CheckSequenceLocks(const CTxMemPool& pool, const CTransaction& tx, int flag
 314 |  // Returns the script flags which should be checked for a given block
 315 |  static unsigned int GetBlockScriptFlags(const CBlockIndex* pindex, const Consensus::Params& chainparams);
 316 |  
 317 | -static void LimitMempoolSize(CTxMemPool& pool, size_t limit, unsigned long age) EXCLUSIVE_LOCKS_REQUIRED(pool.cs)
 318 | +static void LimitMempoolSize(CTxMemPool& pool, size_t limit, unsigned long age)
 319 | +    EXCLUSIVE_LOCKS_REQUIRED(pool.cs, ::cs_main)
    ariard commented at 12:09 AM on July 31, 2019:

    Based on above discussion on lock annotations, you may use also AssertLockHeld here and in UpdateTip (also AcceptToMemoryPoolWithTime, DisconnectTip, ConnectTip)

  56. ariard approved
  57. ariard commented at 12:18 AM on July 31, 2019: member

    ACK 4e307e5 (reviewed changes since last one and ran tests)

  58. MarcoFalke commented at 12:08 PM on July 31, 2019: member

    ACK 4e307e5f5d676d01a94e1b645dcb0ee8574faf77 (only checked that my name is in the credits, did not look at the changes)

    <details><summary>Show signature and timestamp</summary>

    Signature:

    -----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

ACK 4e307e5f5d676d01a94e1b645dcb0ee8574faf77 (only checked that my name is in the credits, did not look at the changes)
-----BEGIN PGP SIGNATURE-----

iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
pUhusgwAu958HvbN9frbzwlJBJbL9+PABYM8kmB/+hArsWHs5J9vxPgDIyeGCsGN
fma8zMP/2Ru+YNBkr8fI8Dw24W7yrr+HliAzwV5czY9RFH+BJa4lON9lxs8Ni+07
cMwQe0I1mYYerOK8GEwmFqluyUpRx9EI5FF0gFsrmz4zKJI5KTGYv1XZ2ILLV4we
Zf3c58H0xLxi2RUywUIqNvYSYqsuJS1hf9bsvVnyzeJh0Uq84o74qnC9Y7Hx3Uy4
4gd4IzxUpiAeToE8KNpPfT2T+DmHNiuREqqmezUOdWkI1DN/RYY/jDORr2CysTvt
7mrrSpmNxbfTiboHZZvpxlFb94zQfxW08n9ex76z1N6aknbu7sTVW62Z+yQKU0LR
Nmcmxrg7gQ5uJU3B/cciM7TpJtHsOWHyUu1gDHGGBluK9kqTBt6jVU+XCwuJAUrU
hOgzIbf4qqIY3Bb5rq04RN/QBTDRZr43by8cX1HmyXNYXWaPQ2ZAVTdIgwYIPkjl
LLr6U7+n
=Lsbq
-----END PGP SIGNATURE-----

    Timestamp of file with hash 055fc53ddab45b0500c4174e6690d5cf015f456b1806475966f66a2d74e0de0c -

    </details>

  59. in src/init.cpp:1545 in 4e307e5f5d outdated 
    1547 |                      strLoadError = _("Unable to replay blocks. You will need to rebuild the database using -reindex-chainstate.").translated;
1548 |                      break;
1549 |                  }
1550 |  
1551 | -                // The on-disk coinsdb is now in a good state, create the cache
1552 | -                pcoinsTip.reset(new CCoinsViewCache(pcoinscatcher.get()));
    MarcoFalke commented at 12:20 PM on July 31, 2019:

    Again, would be nice if you explained why it is safe to flush the db when we were unable to replay the blocks or upgrading the db, ... $any_other_error

    jamesob commented at 6:04 PM on August 6, 2019:

    Good point. Instead of changing any of the init ordering and explaining with a comment, I've made further changes that split up initialization of the coinsdb and coinscache.

  60. DrahtBot added the label Needs rebase on Jul 31, 2019
  61. jamesob force-pushed on Aug 6, 2019
  62. refactor: pcoinsTip -> CChainState::CoinsTip()
    This aliasing makes subsequent commits easier to review; eventually CoinsTip()
will return the CCoinsViewCache managed by CChainState.
    fae6ab6aed
  63. jamesob force-pushed on Aug 6, 2019
  64. jamesob commented at 6:08 PM on August 6, 2019: member

    I've pushed changes (au.coins-under-chainstate.9 -> au.coins-under-chainstate.10) that allow separate initialization of the coinsdb and coinscache so that we can follow existing init semantics without having to think about whether flushing to disk after some kind of database initialization problem is safe or not. I then rebased onto master.

  65. DrahtBot removed the label Needs rebase on Aug 6, 2019
  66. laanwj added this to the "Blockers" column in a project
  67. ryanofsky approved
  68. ryanofsky commented at 7:02 PM on August 12, 2019: member

    utACK cdf6afbbf1e15710ebbd26c02ccbd834db1431de. Changes since last review: rebase, locking cs_main while flushing during shutdown in third commit, adding new asserts to check initialization order, renaming some methods, and splitting coinsdb / coinscache initialization as described in second commit.

  69. in src/validation.h:750 in 087bdf7190 outdated 
     745 | @@ -668,8 +746,10 @@ CChain& ChainActive();
 746 |  /** @returns the global block index map. */
 747 |  BlockMap& BlockIndex();
 748 |  
 749 | -/** Global variable that points to the coins database (protected by cs_main) */
 750 | -extern std::unique_ptr<CCoinsViewDB> pcoinsdbview;
 751 | +// Most often ::ChainstateActive() should be used instead of this, but some code
 752 | +// may not be able to assume that this has been initialized yet and so must use it
 753 | +// directly, e.g. init.cpp.
 754 | +extern std::unique_ptr<CChainState> g_chainstate;
    Sjors commented at 2:13 PM on August 13, 2019:

    Note to other reviewers: g_chainstate is proposed to be retired in bf4a50371448d4a995ccd8e0cf299986b96491fe of the parent PR, by introducing a ChainstateManager. IIUC the current code would not respond well to having more than one CChainState instance.

  70. Sjors approved
  71. Sjors commented at 3:08 PM on August 13, 2019: member

    Code review ACK cdf6afbbf1e15710ebbd26c02ccbd834db1431de. I also lightly stress tested it by quitting QT while rolling back and forward (with --enable-debug).

    Maybe @promag (given his work on #15997) can comment on the last commit that adds lock annotations.

  72. TheBlueMatt commented at 7:38 PM on August 14, 2019: member

    Does it make sense to go ahead and do some kind of ::ChainstateActive().CoinsTip(I_WANT_THE_FULLY_VALIDATED_ONE_ONLY vs IM_OK_WITH_ASSUMEVALID) distinction in this pr?

  73. MarcoFalke commented at 7:51 PM on August 14, 2019: member

    Why would you need such a distinction at the coins tip level? I'd rather have a ::ChainstateBackground() global.

  74. jamesob commented at 8:37 PM on August 14, 2019: member

    @TheBlueMatt no - right now there's no notion of an assumeutxo-valid chainstate. Later on when we want to acknowledge the possibility of multiple chainstates we'll do so through the ChainstateManager interface (as @Sjors noted).

  75. in src/init.cpp:239 in cdf6afbbf1 outdated 
     234 | @@ -235,8 +235,11 @@ void Shutdown(InitInterfaces& interfaces)
 235 |      //
 236 |      // g_chainstate is referenced here directly (instead of ::ChainstateActive()) because it
 237 |      // may not have been initialized yet.
 238 | -    if (g_chainstate && g_chainstate->CanFlushToDisk()) {
 239 | -        g_chainstate->ForceFlushStateToDisk();
 240 | +    {
 241 | +        LOCK(cs_main);
    ariard commented at 9:24 PM on August 14, 2019:

    Don't know about this lock, there is no thread which reset g_chainstate, and if we shutdown before we init g_chainstate we should be still single-threaded. Anyway, taking a lock at shutdown shouldn't hurt

    jamesob commented at 3:00 PM on August 15, 2019:

    Yeah, only added this to satisfy the new lock annotations on CanFlushToDisk().

  76. in src/validation.h:531 in 087bdf7190 outdated 
     528 | +
 529 | +    //! This is the top layer of the cache hierarchy - it keeps as many coins in memory as
 530 | +    //! can fit per the dbcache setting.
 531 | +    std::unique_ptr<CCoinsViewCache> m_cacheview;
 532 | +
 533 | +    //! This constructor initializes CCoinsViewDB and CCoinsViewErrorCatcher instances, but it
    ariard commented at 9:29 PM on August 14, 2019:

    nit: if you rebase, you may add a reference to this in commit message, at would ease understanding of changed since last time :)

    jamesob commented at 3:21 PM on August 15, 2019:

    Updated the message, thanks.

  77. ariard approved
  78. ariard commented at 9:30 PM on August 14, 2019: member

    ACK cdf6afb, reviewed changes since last time and run tests with --enable-debug for lock checking.

  79. in src/init.cpp:1549 in cdf6afbbf1 outdated 
    1551 |                      break;
1552 |                  }
1553 |  
1554 | -                // The on-disk coinsdb is now in a good state, create the cache
1555 | -                pcoinsTip.reset(new CCoinsViewCache(pcoinscatcher.get()));
1556 | +                // The on-disk coinsdb is now in a good state, create the cache.
    MarcoFalke commented at 1:30 PM on August 15, 2019:

    in commit 087bdf7190 refactor: have CCoins* data managed under CChainState

    Why is this comment changed? None of the other comments have trailing dots

  80. in src/validation.h:525 in cdf6afbbf1 outdated 
     520 | +    //! The lowest level of the CoinsViews cache hierarchy sits in a leveldb database on disk.
 521 | +    //! All unspent coins reside in this store.
 522 | +    std::unique_ptr<CCoinsViewDB> m_dbview GUARDED_BY(cs_main);
 523 | +
 524 | +    //! This view wraps access to the leveldb instance and handles read errors gracefully.
 525 | +    std::unique_ptr<CCoinsViewErrorCatcher> m_catcherview GUARDED_BY(cs_main);
    MarcoFalke commented at 1:43 PM on August 15, 2019:

    In commit 087bdf7190 refactor: have CCoins* data managed under CChainState

    Why is this a pointer?

  81. in src/validation.h:522 in cdf6afbbf1 outdated 
     517 | +class CoinsViews {
 518 | +
 519 | +public:
 520 | +    //! The lowest level of the CoinsViews cache hierarchy sits in a leveldb database on disk.
 521 | +    //! All unspent coins reside in this store.
 522 | +    std::unique_ptr<CCoinsViewDB> m_dbview GUARDED_BY(cs_main);
    MarcoFalke commented at 1:43 PM on August 15, 2019:

    Same

  82. in src/validation.h:635 in cdf6afbbf1 outdated 
     631 |  
 632 | +    //! @returns A reference to the in-memory cache of the UTXO set.
 633 | +    CCoinsViewCache& CoinsTip() EXCLUSIVE_LOCKS_REQUIRED(cs_main)
 634 | +    {
 635 | +        assert(m_coins_views->m_cacheview);
 636 | +        return *m_coins_views->m_cacheview.get();
    MarcoFalke commented at 1:44 PM on August 15, 2019:

    in commit 087bdf7190 refactor: have CCoins* data managed under CChainState

    No need for .get();

    (Same below)

  83. in src/validation.cpp:1057 in cdf6afbbf1 outdated 
    1053 | @@ -1053,7 +1054,7 @@ CoinsViews::CoinsViews(
1054 |                              GetDataDir() / ldb_name, cache_size_bytes, in_memory, should_wipe)),
1055 |                          m_catcherview(MakeUnique<CCoinsViewErrorCatcher>(m_dbview.get())) {}
1056 |  
1057 | -void CoinsViews::InitCache()
1058 | +void CoinsViews::InitCache() EXCLUSIVE_LOCKS_REQUIRED(::cs_main)
    MarcoFalke commented at 1:47 PM on August 15, 2019:

    in commit cdf6afbbf1

    Please don't add lock annotations in the cpp file. They will overwrite the ones in the header file

    jamesob commented at 2:52 PM on August 15, 2019:

    Oops, stupid mistake.

  84. in src/validation.cpp:1077 in cdf6afbbf1 outdated 
    1073 | @@ -1073,7 +1074,7 @@ void CChainState::InitCoinsDB(
1074 |          leveldb_name, cache_size_bytes, in_memory, should_wipe);
1075 |  }
1076 |  
1077 | -void CChainState::InitCoinsCache()
1078 | +void CChainState::InitCoinsCache() EXCLUSIVE_LOCKS_REQUIRED(::cs_main)
    MarcoFalke commented at 1:47 PM on August 15, 2019:

    Same

  85. MarcoFalke commented at 1:48 PM on August 15, 2019: member

    partial ACK 087bdf7190

    <details><summary>Show signature and timestamp</summary>

    Signature:

    -----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

ACK 087bdf7190
-----BEGIN PGP SIGNATURE-----

iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
pUiSqQv+PyvXoFWuFIO8A5dCEelRFbODvV6DCXgYAdsWbKLeJQL9djcqnVE1xNHS
XI90uua6p2kjXYOxjDn9l4r3mGizgYJkbiJmjwJef++U8sjGAPhhlxnPvS3X0UMU
83qLml/tGeIWdW/ynlYq/wSQlADMyKfIA6QYyTHlNAN0IPKsVnPtN/RhAJmuyr+z
OvegeJQ+DFJB+Kc7z5SNx0I/c29K0FJhISZkTpGAaeA/fIfyg2p/xIcLnYbLzGnd
m4dsNiklqNJRln3I0qdctuBpX3NCcVOOPfAPW5bJvIGtVbSObY7d09eNqOiS98h7
HRCOLN/MSccoUpXSSb+IdAnVHG+333cKnOSbMzdmsRc6Jtytgc3qTzFtv9MWjoty
GzmLEIchjLhDPOFCzYDb38kkIRNQgvCg+ldfQ7TBuUz76oXGz/kSNuve+V6gzL61
tJOyiW/JTCXJEOEAOTzPfDpybNqIcfkca0bfwOhRnBgC7iJ3U/fRCW2eVuSWryFS
Knp5/0Pk
=w+eT
-----END PGP SIGNATURE-----

    Timestamp of file with hash 0a4a6fbe53dfa1d0e95a1dcea6c07b8d89dd49766c5459918ae9ed368258510d -

    </details>

  86. refactor: have CCoins* data managed under CChainState
    This change encapsulates UTXO set data within CChainState instances, removing
global data `pcoinsTip` and `pcoinsviewdb`. This is necessary if we want to
maintain multiple chainstates with their own rendering of the UTXO set.

We introduce a class CoinsViews which consolidates the construction of a
CCoins* hierarchy. Construction of its various pieces (db, coinscatcher,
in-memory cache) is split up so that we avoid flushing bad state to disk if
startup is interrupted.

We also introduce `CChainState::CanFlushToDisk()` which tells us when it is
safe to flush the chainstate based on this partial construction.

This commit could be broken into smaller pieces, but it would require more
ephemeral diffs to, e.g., temporarily change CCoinsViewDB's constructor
invocations.

Other changes:

- A parameter has been added to the CCoinsViewDB constructor that allows the
  name of the corresponding leveldb directory to be specified.

Thanks to Russell Yanofsky and Marco Falke for helpful feedback.
    5693530685
  87. jamesob force-pushed on Aug 15, 2019
  88. Cover UTXO set access with lock annotations
    i.e. any CoinsViews members. Adds a lock acquisition to `gettxoutsetinfo` RPC
to comply with added annotations.

Co-authored-by: Russell Yanofsky <russ@yanofsky.org>
    582d2cd747
  89. jamesob force-pushed on Aug 15, 2019
  90. jamesob commented at 3:21 PM on August 15, 2019: member

    au.coins-under-chainstate.10 -> au.coins-under-chainstate.12

    Thanks for finding that stuff @MarcoFalke; fixes pushed.

  91. Sjors commented at 4:18 PM on August 15, 2019: member

    reACK 582d2cd74754d6b9a2394616a9c82a89d2d71976

  92. MarcoFalke commented at 4:46 PM on August 15, 2019: member

    ACK 582d2cd747

    <details><summary>Show signature and timestamp</summary>

    Signature:

    -----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

ACK 582d2cd747
-----BEGIN PGP SIGNATURE-----

iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
pUinvAv+MV+6LkNCYGqrYG1HI3cm/fQYw8DmpC0nsUHkaEDHCfaqTvAimUoXxveq
zIb2gsCgbyujAM+zCTLlUBdE1v4stJ1fPMq57afluQNpEXrRSYttdy33YAI5D4Dy
QJ/Ny9kHTdTMukYe8eVnDqNWgwIgjfPkzFrhOw3vlN45e6aVQl7WX+yvaaB5v1/A
fg8eZ93c8ey4X0P6hDaT2jdVNNWidlnXZ2+bbZOqEKUfI0boJtQrcp4oxUkJ40kf
A7kFuwBvKyxUs8HWLeTUW4P39RHBSuhjewaHuBVneJpPOw7I3tYcOUtZS7Rv8CJC
ZZnj+GVLnLyAz0HMD/KCuLGpFV28oxgHdMfqRkNfGvFJFkAsEfyuFjHjWBe9DT9A
owlq2UGeBlMznMC2UK+GIX+5TIKDBShgoXbblu9x8x7R+s9iSRHmEiSEGtOst6qP
OU6mr2hOTB3qg1wywWHNbxfqpJiuP6TknWOFm7G5GnJcSrKUfO42RII0Q2E/oVQ2
QxgRxcan
=/V5U
-----END PGP SIGNATURE-----

    Timestamp of file with hash 8c58c7b1c21f3568020ef04ac438a92aba830446a5172e171b37e02c744d64e0 -

    </details>

  93. MarcoFalke merged this on Aug 15, 2019
  94. MarcoFalke closed this on Aug 15, 2019
  95. MarcoFalke referenced this in commit 85883a9f8e on Aug 15, 2019
  96. laanwj removed this from the "Blockers" column in a project
  97. fanquake moved this from the "In progress" to the "Done" column in a project
  98. domob1812 referenced this in commit 10596042b0 on Aug 19, 2019
  99. jasonbcox referenced this in commit 9c533d5924 on Sep 24, 2020
  100. jasonbcox referenced this in commit cc35ed039d on Oct 1, 2020
  101. jasonbcox referenced this in commit 05e8603926 on Oct 15, 2020
  102. jasonbcox referenced this in commit 76953e642b on Oct 16, 2020
  103. jasonbcox referenced this in commit 74ade9cfe6 on Oct 16, 2020
  104. jasonbcox referenced this in commit 124054fe2d on Oct 16, 2020
  105. kittywhiskers referenced this in commit 1bb2d2b2a6 on Oct 10, 2021
  106. kittywhiskers referenced this in commit 0384ec26e0 on Oct 10, 2021
  107. kittywhiskers referenced this in commit 1617b7cb51 on Oct 16, 2021
  108. kittywhiskers referenced this in commit a69c771ea8 on Oct 16, 2021
  109. kittywhiskers referenced this in commit ff2209126d on Oct 16, 2021
  110. kittywhiskers referenced this in commit 96ccf93f48 on Oct 21, 2021
  111. kittywhiskers referenced this in commit 84339a4b84 on Oct 22, 2021
  112. pravblockc referenced this in commit 1229d63d57 on Nov 18, 2021
  113. DrahtBot locked this on Dec 16, 2021
Contributors
jamesobMarcoFalkeryanofskyDrahtBotariardsipaSjorsTheBlueMatt
Labels
Refactoring
Linked (view graph)
#15605 assumeutxo#15606 assumeutxo
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-04-22 00:14 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me