Skip to content

descriptor: speed-up Parse (xpub/xpriv) in ~30%#23

Open
brunoerg wants to merge 696 commits into
masterfrom
2026-04-descriptor
Open

descriptor: speed-up Parse (xpub/xpriv) in ~30%#23
brunoerg wants to merge 696 commits into
masterfrom
2026-04-descriptor

Conversation

@brunoerg

@brunoerg brunoerg commented Apr 8, 2026

Copy link
Copy Markdown
Owner

This PR speeds up descriptor parsing, for basically descriptors that use xpub/xpriv, through some targeted improvements:

  • Avoid redundant base58 decoding when parsing extended keys (this is the major improvement)

    ParsePubkeyInner unconditionally called both DecodeExtKey() and DecodeExtPubKey(), each running a full DecodeBase58Check (two SHA256 passes over ~111 bytes). A new DecodeExtKeyOrPubKey() function performs a single base58 decode and then checks the prefix, halving the cryptographic work per key. The interfaces of DecodeSecret, DecodeExtKey, and DecodeExtPubKey are also updated to accept string_view instead of const std::string&, eliminating unnecessary string copies in the descriptor parser and allowing DecodeBase58/DecodeBase58Check to use pointer-range iteration instead of strlen.

  • Replace O(n) charset lookup with precomputed table

    DescriptorChecksum previously called std::string::find() for every character in the descriptor string, doing up to 96 comparisons per character. This replaces it with a 256-entry precomputed lookup table for O(1) lookups. The benefit scales with descriptor length.


Parse() with the following descriptor:

tr(xpub6CGCzpvrZVssNNXuP1Awcz8GoakYyJHra2WHASXfCzsD9szbQFoT1pLMdQopyqaSqXUprH7fkk8NG2pUbQa5HpNzpWmEsdNZDHxXvSLPTrn/*,{{multi_a(20,xpub6CGCzpvrZVssNNXuP1Awcz8GoakYyJHra2WHASXfCzsD9szbQFoT1pLMdQopyqaSqXUprH7fkk8NG2pUbQa5HpNzpWmEsdNZDHxXvSLPTrn/*,xpub6C5MW95EE8hAmxm96LdAFYkweaAktQoojCf2MwJmNL9Fi6oUPeBnpFdEzhs9zHwr4HQEeF1r1yAUR8MqDJqk4YPteh75rNoCKCtud7shDDk/*,xpub6DNEUJP1zLAZ1P6gYRba9FyxuppmkrPmSKz3FLM7msRwn2LEr4LnKfAnNoSoAbnbW2orTfsitdfWTxLbJCZ6LizqV7cFUavSAjWzNjqdfCP/*,xpub6Cm5sAgCZQvYtnatYPdFuLhsD9VtgY3Rtt48TrdgqXm4DttXfM6nFmFTko8KcyGhPswTvPcTzfpMUvqV3F9tgLPvzfzoKwjmU668vBmGXEX/*,xpub6DN66R61Luv95HLvznR2ZWsbYuGBoK9LDR6XiNm1pvp5zRJ2vFhLQ4PBe2pDn59sKmRnEFbdLtBVZFN3QAujcGZ5PomGCvrnisxkP2fcgHg/*,xpub6D2GtAtYodmjwEoAzorKUta44NDvAB4JjafYXxsQ5hTWvCAK55nnkyNHfdjrsx3umfgGe1nnHut13mLuFqqi2fs7VVyv6rht76ddrDGhrTY/*,xpub6Bfi8jsDMuu2XHQ1TGRB6ehjdHKKmTwRFQYU6QcgHoZswqbzKsXs9Qd87nAxevjZwuQ37jaVnQoPF6wY34BJoeLxXkEYMpKz2CBHLcEmJVF/*,xpub6CYiNdQesStiBwZkWjPBxwxxf6oPMH9meBMujCwBX74X8nCpMF9zRZUbDrU9raQah1LQcnzLQrHC6Kr6LNCmsBpaMkYD3U6FyhHZfTHQGUB/*,xpub6CddszJHLa3VisZ6ufU4hiFcGsuuPHo7hYa24o3kPuPfAbMED4ajuDa6svuqxxBxYgUQhN8LK7MtTMetquNo7YsWhiV8hX2gBfjszJVXgkG/*,xpub6DAM262AxC4Um3rxBiP5FoA7SJJG3a7MjaJkEyXWyMA6iNuyqWueKZHSSNjKEWnn8Wi3PUQNTV1Z3a59C2Qaidha88GTWtAMhJBAWBBvuTN/*,xpub6ByZcsLsr8xTCJmYxvLpLrsFzuHNXRLR4xMyB9ZFLek1hDVh7zba2xAVTU8M6eonFFAAPYpTA76bDuopF5zyBkZqFc3rtJ3ziH1BUXkjC6j/*,xpub6Bx2LErujvR1noXrk7RHQB9htiDcPM5qYuV7Jg1nZh5jWeK8pYB4DnYQ73EX9sB9M6rnNsF8LDPUejHiXrBx2JJQSdVQ9i9FfEzJLuAnzEd/*,xpub6DSBog1uLngjBWVDUpLTQ19jGGKovgc3iTd1ijL4fjk9sgNqrnG5kGLjUWncay2jbqdgt6yrbYSHhfwsTMp7i8pUD1FMCQuFyqdsZpgUNH8/*,xpub6C53ov8LpuGp5MM4ghqUFJcdTnXJCRHyXaz1RVCys2uQErpdzUWHtm8Bsg8NJyaqfhGWvm9WPSE7jMneCRw1xs1CTajpToXyRqSCc41JiuY/*,xpub6CscWPNXdEybbJVvx7fJTrfHyTkKZtb97qcpQ9fH4k4xtm4nkckSpvy86heQsdyoF28CugwLHg5kLDwpGoPQAYXQ3RG9cJB2Na1FdeN1wt5/*,xpub6Cmrivt6w3kVAxw7NmcKSY19gSLmC2tVwx46cdigr9d1UEnttJCGZJ7NQu1MkugpZgF9rQ3j3NKUiZ2MoaC2QoZUfi4nwxbpH39SQZaBHut/*,xpub6D1Ey5JAYnBxFE8i2V2SUsEpJt7r3ykJKcShcpT2xN9CXtVQy5dKeYiZ9D1bdnkPrmgEajrSuA5nWqf94s6u7FwvXy3cU4BXUMgAt9C8wGT/*,xpub6CwJ2vicnoSJNFDzhonucHNVHDwyb1vyiu2VF6zmjQrzGb4qoAma1i2QJ7qRNe1qmpvvWwzSuQDgMnCECFQhZaPKuPFjcMo33r2xenBauxf/*,xpub6CiLNz8mFB4fB51x6gMNu44Up9PyVhq7fSxQb8UzYQermvMShwWyqzQe99Uiyxpfy1ow42xUGpPGBHf8ErxDbQojBf7Uy7hKFgYk8KpgMm2/*,xpub6DQ2qVMZ2o2QGGpYDsw5hpAcygNVEycAkkCib2Lexwm8gYWMCSecZ256AbAV93Rgze6h8yEoTcqUnvnz7L69WUqp7cXd85X8HoeKWjPHRRZ/*),multi_a(20,xpub6CGCzpvrZVssNNXuP1Awcz8GoakYyJHra2WHASXfCzsD9szbQFoT1pLMdQopyqaSqXUprH7fkk8NG2pUbQa5HpNzpWmEsdNZDHxXvSLPTrn/*,xpub6C5MW95EE8hAmxm96LdAFYkweaAktQoojCf2MwJmNL9Fi6oUPeBnpFdEzhs9zHwr4HQEeF1r1yAUR8MqDJqk4YPteh75rNoCKCtud7shDDk/*,xpub6DNEUJP1zLAZ1P6gYRba9FyxuppmkrPmSKz3FLM7msRwn2LEr4LnKfAnNoSoAbnbW2orTfsitdfWTxLbJCZ6LizqV7cFUavSAjWzNjqdfCP/*,xpub6Cm5sAgCZQvYtnatYPdFuLhsD9VtgY3Rtt48TrdgqXm4DttXfM6nFmFTko8KcyGhPswTvPcTzfpMUvqV3F9tgLPvzfzoKwjmU668vBmGXEX/*,xpub6DN66R61Luv95HLvznR2ZWsbYuGBoK9LDR6XiNm1pvp5zRJ2vFhLQ4PBe2pDn59sKmRnEFbdLtBVZFN3QAujcGZ5PomGCvrnisxkP2fcgHg/*,xpub6D2GtAtYodmjwEoAzorKUta44NDvAB4JjafYXxsQ5hTWvCAK55nnkyNHfdjrsx3umfgGe1nnHut13mLuFqqi2fs7VVyv6rht76ddrDGhrTY/*,xpub6Bfi8jsDMuu2XHQ1TGRB6ehjdHKKmTwRFQYU6QcgHoZswqbzKsXs9Qd87nAxevjZwuQ37jaVnQoPF6wY34BJoeLxXkEYMpKz2CBHLcEmJVF/*,xpub6CYiNdQesStiBwZkWjPBxwxxf6oPMH9meBMujCwBX74X8nCpMF9zRZUbDrU9raQah1LQcnzLQrHC6Kr6LNCmsBpaMkYD3U6FyhHZfTHQGUB/*,xpub6CddszJHLa3VisZ6ufU4hiFcGsuuPHo7hYa24o3kPuPfAbMED4ajuDa6svuqxxBxYgUQhN8LK7MtTMetquNo7YsWhiV8hX2gBfjszJVXgkG/*,xpub6DAM262AxC4Um3rxBiP5FoA7SJJG3a7MjaJkEyXWyMA6iNuyqWueKZHSSNjKEWnn8Wi3PUQNTV1Z3a59C2Qaidha88GTWtAMhJBAWBBvuTN/*,xpub6ByZcsLsr8xTCJmYxvLpLrsFzuHNXRLR4xMyB9ZFLek1hDVh7zba2xAVTU8M6eonFFAAPYpTA76bDuopF5zyBkZqFc3rtJ3ziH1BUXkjC6j/*,xpub6Bx2LErujvR1noXrk7RHQB9htiDcPM5qYuV7Jg1nZh5jWeK8pYB4DnYQ73EX9sB9M6rnNsF8LDPUejHiXrBx2JJQSdVQ9i9FfEzJLuAnzEd/*,xpub6DSBog1uLngjBWVDUpLTQ19jGGKovgc3iTd1ijL4fjk9sgNqrnG5kGLjUWncay2jbqdgt6yrbYSHhfwsTMp7i8pUD1FMCQuFyqdsZpgUNH8/*,xpub6C53ov8LpuGp5MM4ghqUFJcdTnXJCRHyXaz1RVCys2uQErpdzUWHtm8Bsg8NJyaqfhGWvm9WPSE7jMneCRw1xs1CTajpToXyRqSCc41JiuY/*,xpub6CscWPNXdEybbJVvx7fJTrfHyTkKZtb97qcpQ9fH4k4xtm4nkckSpvy86heQsdyoF28CugwLHg5kLDwpGoPQAYXQ3RG9cJB2Na1FdeN1wt5/*,xpub6Cmrivt6w3kVAxw7NmcKSY19gSLmC2tVwx46cdigr9d1UEnttJCGZJ7NQu1MkugpZgF9rQ3j3NKUiZ2MoaC2QoZUfi4nwxbpH39SQZaBHut/*,xpub6D1Ey5JAYnBxFE8i2V2SUsEpJt7r3ykJKcShcpT2xN9CXtVQy5dKeYiZ9D1bdnkPrmgEajrSuA5nWqf94s6u7FwvXy3cU4BXUMgAt9C8wGT/*,xpub6CwJ2vicnoSJNFDzhonucHNVHDwyb1vyiu2VF6zmjQrzGb4qoAma1i2QJ7qRNe1qmpvvWwzSuQDgMnCECFQhZaPKuPFjcMo33r2xenBauxf/*,xpub6CiLNz8mFB4fB51x6gMNu44Up9PyVhq7fSxQb8UzYQermvMShwWyqzQe99Uiyxpfy1ow42xUGpPGBHf8ErxDbQojBf7Uy7hKFgYk8KpgMm2/*,xpub6DQ2qVMZ2o2QGGpYDsw5hpAcygNVEycAkkCib2Lexwm8gYWMCSecZ256AbAV93Rgze6h8yEoTcqUnvnz7L69WUqp7cXd85X8HoeKWjPHRRZ/*)},{multi_a(20,xpub6CGCzpvrZVssNNXuP1Awcz8GoakYyJHra2WHASXfCzsD9szbQFoT1pLMdQopyqaSqXUprH7fkk8NG2pUbQa5HpNzpWmEsdNZDHxXvSLPTrn/*,xpub6C5MW95EE8hAmxm96LdAFYkweaAktQoojCf2MwJmNL9Fi6oUPeBnpFdEzhs9zHwr4HQEeF1r1yAUR8MqDJqk4YPteh75rNoCKCtud7shDDk/*,xpub6DNEUJP1zLAZ1P6gYRba9FyxuppmkrPmSKz3FLM7msRwn2LEr4LnKfAnNoSoAbnbW2orTfsitdfWTxLbJCZ6LizqV7cFUavSAjWzNjqdfCP/*,xpub6Cm5sAgCZQvYtnatYPdFuLhsD9VtgY3Rtt48TrdgqXm4DttXfM6nFmFTko8KcyGhPswTvPcTzfpMUvqV3F9tgLPvzfzoKwjmU668vBmGXEX/*,xpub6DN66R61Luv95HLvznR2ZWsbYuGBoK9LDR6XiNm1pvp5zRJ2vFhLQ4PBe2pDn59sKmRnEFbdLtBVZFN3QAujcGZ5PomGCvrnisxkP2fcgHg/*,xpub6D2GtAtYodmjwEoAzorKUta44NDvAB4JjafYXxsQ5hTWvCAK55nnkyNHfdjrsx3umfgGe1nnHut13mLuFqqi2fs7VVyv6rht76ddrDGhrTY/*,xpub6Bfi8jsDMuu2XHQ1TGRB6ehjdHKKmTwRFQYU6QcgHoZswqbzKsXs9Qd87nAxevjZwuQ37jaVnQoPF6wY34BJoeLxXkEYMpKz2CBHLcEmJVF/*,xpub6CYiNdQesStiBwZkWjPBxwxxf6oPMH9meBMujCwBX74X8nCpMF9zRZUbDrU9raQah1LQcnzLQrHC6Kr6LNCmsBpaMkYD3U6FyhHZfTHQGUB/*,xpub6CddszJHLa3VisZ6ufU4hiFcGsuuPHo7hYa24o3kPuPfAbMED4ajuDa6svuqxxBxYgUQhN8LK7MtTMetquNo7YsWhiV8hX2gBfjszJVXgkG/*,xpub6DAM262AxC4Um3rxBiP5FoA7SJJG3a7MjaJkEyXWyMA6iNuyqWueKZHSSNjKEWnn8Wi3PUQNTV1Z3a59C2Qaidha88GTWtAMhJBAWBBvuTN/*,xpub6ByZcsLsr8xTCJmYxvLpLrsFzuHNXRLR4xMyB9ZFLek1hDVh7zba2xAVTU8M6eonFFAAPYpTA76bDuopF5zyBkZqFc3rtJ3ziH1BUXkjC6j/*,xpub6Bx2LErujvR1noXrk7RHQB9htiDcPM5qYuV7Jg1nZh5jWeK8pYB4DnYQ73EX9sB9M6rnNsF8LDPUejHiXrBx2JJQSdVQ9i9FfEzJLuAnzEd/*,xpub6DSBog1uLngjBWVDUpLTQ19jGGKovgc3iTd1ijL4fjk9sgNqrnG5kGLjUWncay2jbqdgt6yrbYSHhfwsTMp7i8pUD1FMCQuFyqdsZpgUNH8/*,xpub6C53ov8LpuGp5MM4ghqUFJcdTnXJCRHyXaz1RVCys2uQErpdzUWHtm8Bsg8NJyaqfhGWvm9WPSE7jMneCRw1xs1CTajpToXyRqSCc41JiuY/*,xpub6CscWPNXdEybbJVvx7fJTrfHyTkKZtb97qcpQ9fH4k4xtm4nkckSpvy86heQsdyoF28CugwLHg5kLDwpGoPQAYXQ3RG9cJB2Na1FdeN1wt5/*,xpub6Cmrivt6w3kVAxw7NmcKSY19gSLmC2tVwx46cdigr9d1UEnttJCGZJ7NQu1MkugpZgF9rQ3j3NKUiZ2MoaC2QoZUfi4nwxbpH39SQZaBHut/*,xpub6D1Ey5JAYnBxFE8i2V2SUsEpJt7r3ykJKcShcpT2xN9CXtVQy5dKeYiZ9D1bdnkPrmgEajrSuA5nWqf94s6u7FwvXy3cU4BXUMgAt9C8wGT/*,xpub6CwJ2vicnoSJNFDzhonucHNVHDwyb1vyiu2VF6zmjQrzGb4qoAma1i2QJ7qRNe1qmpvvWwzSuQDgMnCECFQhZaPKuPFjcMo33r2xenBauxf/*,xpub6CiLNz8mFB4fB51x6gMNu44Up9PyVhq7fSxQb8UzYQermvMShwWyqzQe99Uiyxpfy1ow42xUGpPGBHf8ErxDbQojBf7Uy7hKFgYk8KpgMm2/*,xpub6DQ2qVMZ2o2QGGpYDsw5hpAcygNVEycAkkCib2Lexwm8gYWMCSecZ256AbAV93Rgze6h8yEoTcqUnvnz7L69WUqp7cXd85X8HoeKWjPHRRZ/*),multi_a(20,xpub6CGCzpvrZVssNNXuP1Awcz8GoakYyJHra2WHASXfCzsD9szbQFoT1pLMdQopyqaSqXUprH7fkk8NG2pUbQa5HpNzpWmEsdNZDHxXvSLPTrn/*,xpub6C5MW95EE8hAmxm96LdAFYkweaAktQoojCf2MwJmNL9Fi6oUPeBnpFdEzhs9zHwr4HQEeF1r1yAUR8MqDJqk4YPteh75rNoCKCtud7shDDk/*,xpub6DNEUJP1zLAZ1P6gYRba9FyxuppmkrPmSKz3FLM7msRwn2LEr4LnKfAnNoSoAbnbW2orTfsitdfWTxLbJCZ6LizqV7cFUavSAjWzNjqdfCP/*,xpub6Cm5sAgCZQvYtnatYPdFuLhsD9VtgY3Rtt48TrdgqXm4DttXfM6nFmFTko8KcyGhPswTvPcTzfpMUvqV3F9tgLPvzfzoKwjmU668vBmGXEX/*,xpub6DN66R61Luv95HLvznR2ZWsbYuGBoK9LDR6XiNm1pvp5zRJ2vFhLQ4PBe2pDn59sKmRnEFbdLtBVZFN3QAujcGZ5PomGCvrnisxkP2fcgHg/*,xpub6D2GtAtYodmjwEoAzorKUta44NDvAB4JjafYXxsQ5hTWvCAK55nnkyNHfdjrsx3umfgGe1nnHut13mLuFqqi2fs7VVyv6rht76ddrDGhrTY/*,xpub6Bfi8jsDMuu2XHQ1TGRB6ehjdHKKmTwRFQYU6QcgHoZswqbzKsXs9Qd87nAxevjZwuQ37jaVnQoPF6wY34BJoeLxXkEYMpKz2CBHLcEmJVF/*,xpub6CYiNdQesStiBwZkWjPBxwxxf6oPMH9meBMujCwBX74X8nCpMF9zRZUbDrU9raQah1LQcnzLQrHC6Kr6LNCmsBpaMkYD3U6FyhHZfTHQGUB/*,xpub6CddszJHLa3VisZ6ufU4hiFcGsuuPHo7hYa24o3kPuPfAbMED4ajuDa6svuqxxBxYgUQhN8LK7MtTMetquNo7YsWhiV8hX2gBfjszJVXgkG/*,xpub6DAM262AxC4Um3rxBiP5FoA7SJJG3a7MjaJkEyXWyMA6iNuyqWueKZHSSNjKEWnn8Wi3PUQNTV1Z3a59C2Qaidha88GTWtAMhJBAWBBvuTN/*,xpub6ByZcsLsr8xTCJmYxvLpLrsFzuHNXRLR4xMyB9ZFLek1hDVh7zba2xAVTU8M6eonFFAAPYpTA76bDuopF5zyBkZqFc3rtJ3ziH1BUXkjC6j/*,xpub6Bx2LErujvR1noXrk7RHQB9htiDcPM5qYuV7Jg1nZh5jWeK8pYB4DnYQ73EX9sB9M6rnNsF8LDPUejHiXrBx2JJQSdVQ9i9FfEzJLuAnzEd/*,xpub6DSBog1uLngjBWVDUpLTQ19jGGKovgc3iTd1ijL4fjk9sgNqrnG5kGLjUWncay2jbqdgt6yrbYSHhfwsTMp7i8pUD1FMCQuFyqdsZpgUNH8/*,xpub6C53ov8LpuGp5MM4ghqUFJcdTnXJCRHyXaz1RVCys2uQErpdzUWHtm8Bsg8NJyaqfhGWvm9WPSE7jMneCRw1xs1CTajpToXyRqSCc41JiuY/*,xpub6CscWPNXdEybbJVvx7fJTrfHyTkKZtb97qcpQ9fH4k4xtm4nkckSpvy86heQsdyoF28CugwLHg5kLDwpGoPQAYXQ3RG9cJB2Na1FdeN1wt5/*,xpub6Cmrivt6w3kVAxw7NmcKSY19gSLmC2tVwx46cdigr9d1UEnttJCGZJ7NQu1MkugpZgF9rQ3j3NKUiZ2MoaC2QoZUfi4nwxbpH39SQZaBHut/*,xpub6D1Ey5JAYnBxFE8i2V2SUsEpJt7r3ykJKcShcpT2xN9CXtVQy5dKeYiZ9D1bdnkPrmgEajrSuA5nWqf94s6u7FwvXy3cU4BXUMgAt9C8wGT/*,xpub6CwJ2vicnoSJNFDzhonucHNVHDwyb1vyiu2VF6zmjQrzGb4qoAma1i2QJ7qRNe1qmpvvWwzSuQDgMnCECFQhZaPKuPFjcMo33r2xenBauxf/*,xpub6CiLNz8mFB4fB51x6gMNu44Up9PyVhq7fSxQb8UzYQermvMShwWyqzQe99Uiyxpfy1ow42xUGpPGBHf8ErxDbQojBf7Uy7hKFgYk8KpgMm2/*,xpub6DQ2qVMZ2o2QGGpYDsw5hpAcygNVEycAkkCib2Lexwm8gYWMCSecZ256AbAV93Rgze6h8yEoTcqUnvnz7L69WUqp7cXd85X8HoeKWjPHRRZ/*)}})

This PR:

|               ns/op |                op/s |    err% |     total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
|           90,253.50 |           11,079.90 |    0.2% |      0.01 | `ParseDescriptor`

Master:

|               ns/op |                op/s |    err% |     total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
|          127,812.50 |            7,823.96 |    0.5% |      0.01 | `ParseDescriptor`

Joint work with claude 😅

@brunoerg brunoerg force-pushed the 2026-04-descriptor branch 2 times, most recently from fb7c5b9 to 6b4e303 Compare April 8, 2026 23:03
@brunoerg brunoerg changed the title descriptor: speed-up Parse in ~30% descriptor: speed-up Parse (xpub/xpriv) in ~30% Apr 9, 2026
@brunoerg brunoerg force-pushed the 2026-04-descriptor branch from 6b4e303 to 2d86114 Compare April 9, 2026 17:50
fjahr and others added 26 commits May 15, 2026 23:07
When bigger changes are split across multiple commits, intermediate
commits may introduce unused functions. Do not check for this error
in intermediate commits.
…ig}` functions to `musig.{h,cpp}` module

8ba5f68 refactor, key: move `CreateMuSig2PartialSig` to `musig.{h,cpp}` module (Sebastian Falbesoner)
d087f26 refactor, key: move `CreateMuSig2Nonce` to `musig.{h,cpp}` module (Sebastian Falbesoner)
f36d89f key: add `GetSecp256k1SignContext` access function (w0xlt)

Pull request description:

  This PR is a follow-up of bitcoin#29675, see bitcoin#29675 (comment). It moves all MuSig2 functions that currently live in `CKey` and call secp256k1 musig module API functions (i.e. `secp256k1_musig_...`) to the `musig.{h,cpp}` module, as this seems to be a better place. For accessing the `secp256k1_context_signing` object from the outside, a new function `GetSecp256k1SignContext` is added in the third commit.

  As the patch is mostly move-only, it can be best reviewed via the git option `--color-moved=dimmed-zebra`

ACKs for top commit:
  achow101:
    ACK 8ba5f68
  w0xlt:
    reACK 8ba5f68
  rkrux:
    lgtm ACK 8ba5f68
  furszy:
    ACK 8ba5f68

Tree-SHA512: 95fcaa5d7a09037a0dce0053b8c640a7372a1251a2a3615c565f4dacc5aad5cf0ee8bfc43aa0d0def628465c16330d69f6ea9fcc07bbadc971863248f60d1878
096bb0b bench: add benchmark for GetMappedAS() (0xb10c)

Pull request description:

  With bitcoin#28792 merged, we can use the embedded ASMap file to benchmark the IP -> ASN lookups. Before, this would have been cumbersome as it required having a real, external ASMap file. We want to benchmark against a real file, as a smaller test file has significantly faster lookups.

  The benchmarks cover individual IP address lookups of mapped and unmapped IPv4 and IPv6 addresses along with a multi-IP lookup. For the IPs we assume to be mapped, we assert that they are mapped. Updating the embedded ASMap file might change the benchmark results slightly as some lookups will be a bit faster and others slower.

  ```
  $ ./build/bin/bench_bitcoin --filter=ASMapGetMappedAS.* -min-time=5000

      |               ns/op |                op/s |    err% |     total | benchmark
      |--------------------:|--------------------:|--------:|----------:|:----------
      |              460.41 |        2,171,994.61 |    0.2% |      5.48 | `ASMapGetMappedASCloudflarev4`
      |              356.96 |        2,801,460.45 |    0.0% |      5.33 | `ASMapGetMappedASCloudflarev6`
      |              476.58 |        2,098,304.01 |    0.1% |      5.51 | `ASMapGetMappedASGooglev4`
      |              359.17 |        2,784,224.15 |    0.0% |      5.33 | `ASMapGetMappedASGooglev6`
      |              398.82 |        2,507,410.35 |    0.1% |      5.50 | `ASMapGetMappedASMulti`
      |              346.11 |        2,889,237.31 |    0.3% |      5.32 | `ASMapGetMappedASQuad9v4`
      |              274.21 |        3,646,832.70 |    0.1% |      5.50 | `ASMapGetMappedASQuad9v6`
      |                5.32 |      187,875,440.96 |    0.1% |      5.50 | `ASMapGetMappedASUnmappedv4`
      |               65.19 |       15,339,680.04 |    0.1% |      5.51 | `ASMapGetMappedASUnmappedv6`
  ```

  _LLM disclosure: while I wrote the initial benchmarks, I had a LLM review it and point out style nits and typos to me._

ACKs for top commit:
  l0rinc:
    ACK 096bb0b
  fjahr:
    ACK 096bb0b
  sipa:
    ACK 096bb0b
  achow101:
    ACK 096bb0b

Tree-SHA512: 417427fd546fe7ec0903dde6f1a6557479a2e353eaf26be61dacc0031adae1a9970263b143fe9e2b83f489da01031cc83eff8e11ccedb00f5852e44c40189da2
Document on `btck_chainstate_manager_options_set_wipe_dbs` that a wipe must be followed by `btck_chainstate_manager_import_blocks` before the chainstate manager is used for anything else, as the existing kernel tests already do (e.g. `chainman_reindex_test`).

Note in the `@return` of `btck_chainstate_manager_get_best_entry` that it can return null when no block headers have been loaded.

Refactor the `@return` documentation to fit on a single line.
CombinePSBTs currently preserves unknown records but drops proprietary records at the global, input, and output levels because the Merge() paths never union m_proprietary.

Preserve proprietary records in PartiallySignedTransaction::Merge(), PSBTInput::Merge(), and PSBTOutput::Merge() so combine/merge keeps all PSBT key-value data.
Add unit and functional regression tests asserting that combine/merge preserves proprietary fields at the global, input, and output scopes.
…p include_dummy_extranonce

8544537 mining: drop unused include_dummy_extranonce option (Sjors Provoost)
58eeab7 mining: only pad with OP_0 at heights <= 16 (Sjors Provoost)
00d2232 mining: pad coinbase to fix createNewBlock at heights <=16 (Sjors Provoost)
605ff37 test: bad-cb-length for createNewBlock() at low heights (Sjors Provoost)
1966621 test: refactor IPC mining test to use script_BIP34_coinbase_height (Sjors Provoost)

Pull request description:

  Blocks 0-16 on any new chain require mining code to be careful not to violate the `bad-cb-length` rule, which states the coinbase transaction scriptSig must be at least 2 bytes.

  Our mining code deals with that by padding the `scriptSig` with a 0 `extraNonce`. It does this for every height. As a result IPC clients would get an unnecessary `0` in the `scriptSigPrefix` field of  `CoinbaseTx`. bitcoin#32420 fixed that by introducing a `include_dummy_extranonce` option in `BlockCreateOptions` and turning that off for IPC clients.

  A minor issue was missed though: `createNewBlock()` now fails with `bad-cb-length`. An easy workaround is to use the `generate` RPC for the first 16 blocks, as demonstrated in the 2nd commit.

  The real fix is to have the miner code always pad the `scriptSig` at lower heights, but to _not_ include that in the `scriptSigPrefix` field of  `CoinbaseTx` (introduced in bitcoin#33819). This is what the 3rd commit implements.

  Now that we set `scriptSigPrefix` independent of what our internal miner code does - to get past `CheckBlock()` - the original motivation for `include_dummy_extranonce` goes away and we can just drop it entirely. The last commit drops it, while the 4th commit adjusts the tests and hardcoded block and assume utxo hashes.

  This last change does not break IPC clients, because `include_dummy_extranonce` was never exposed in `mining.capnp`.

  Instead of adjusting the hardcoded hashes, an alternative approach would be to just always pad the `scriptSig` internally, since we exclude the padding from `scriptSigPrefix` anyway. However, IPC clients can also call `getBlock()` to get the raw block and might be confused about the difference. The miner code is also easier to understand if we limit the exception (`coinbase_tx.script_sig_prefix != coinbaseTx.vin[0].scriptSig`) to `nHeight <= 16`, where the explanation is based purely on consensus rules rather than historical test suite reasons.

  The first two commits are preperation test changes:
  -  extract `assert_capnp_failed` helper for macOS (also part of bitcoin#34727)
  -  use `script_BIP34_coinbase_height` in IPC mining test (existing code in `interface_ipc_mining.py` was incorrect for low height

  Fixes bitcoin#35126

ACKs for top commit:
  ryanofsky:
    Code review ACK 8544537. Just rebased to fix silent conflict and applied some minor suggestions since last review. As part of rereviewing I left some more minor suggestions that are fine to ignore.
  sedited:
    Re-ACK 8544537

Tree-SHA512: a01d48842bf4bcc1a9c51a89ef9d750766db7d04edb4dcd6b3a8bf195c6b4fa07445256a49367ff0db00ab489a52a3d7ff6a5c3ab9290ecb1fcb82f532552e9b
b6c3670 i2p: clean up SAM error logging (takeshikurosawaa)

Pull request description:

  Clean up the I2P SAM error path.

  `SESSION CREATE` may contain the private key, so the generic SAM reply
  error path now reports the redacted request text instead of the full
  request. It also avoids echoing raw router replies in those generic
  error messages.

  No network behavior change intended.

ACKs for top commit:
  davidgumberg:
    crACK bitcoin@b6c3670
  vasild:
    ACK b6c3670

Tree-SHA512: 204c8b64c6d3dd2f94f92cdc6d3daefd7773c42066984b9da859ebc2912c2ed38079d9e82a2d1f09d8d720750047114a80189e688929d7a0af5da2c2ee4a88da
… PSBTs

da76985 test: add PSBT proprietary merge regression coverage (w0xlt)
3f5b3c7 psbt: preserve proprietary fields when combining PSBTs (w0xlt)

Pull request description:

  BIP 174 proprietary fields are currently parsed, serialized, and exposed by `decodepsbt`, but they are not preserved by `combinepsbt`.

  The reason is that the merge paths in `PartiallySignedTransaction::Merge()`, `PSBTInput::Merge()`, and `PSBTOutput::Merge()` union `unknown`, but never union `m_proprietary`.

  This means application-specific PSBT metadata can be lost during combination, even though BIP 174 treats proprietary records as normal PSBT key-value pairs for private or application-specific use.

  This PR fixes that by preserving proprietary fields in all three merge paths.

ACKs for top commit:
  nervana21:
    re-ACK da76985
  Bicaru20:
    re-ACK da76985
  achow101:
    ACK da76985
  theStack:
    Code-review ACK da76985

Tree-SHA512: 4474674ac00c3155fd7d3d777bdeb70d4ca2006c8fe62eb84a888ef2f7aa02739bf31d8e9f2cb59e324be1085c77897f0bbf6673254c12408ab09a1582e28b83
89af67d tests: Add some fuzz test coverage for command-specific args (Anthony Towns)
92df785 tests: Add some test coverage for ArgsManager::AddCommand (Anthony Towns)
33c8090 ArgsManager: automate checking for correct command options (Anthony Towns)
186354a bitcoin-wallet: use command-specific options (Anthony Towns)
d21e82b ArgsManager: support command-specific options (Anthony Towns)

Pull request description:

  Adds the ability to link particular options to one or more (`OptionsCategory::COMMANDS`) commands, and uses this feature
  in `bitcoin-wallet`. Separates out the help information for these command-specific options (duplicating it if an option applies to multiple commands), and provides a function for checking at runtime if some options have been specified by the user that only apply to other commands.

  #### Motivation

  Currently, `ArgsManager` supports commands like `bitcoin-wallet dump` but while some of the options are command-specific (like `-dumpfile`), `ArgsManager` itself doesn't know that.  As a result, `-dumpfile` is listed in the global help rather than under the relevant commands, and if you use `-dumpfile` with a different command that doesn't support it, `ArgsManager` cannot automatically report that as an error, resulting in the commands that don't support the option having to have error-handling specific to all the options they don't support.

  #### Changes

  Help output moves command-specific options under their associated commands:

  Before:

  ```
  Options:
    -dumpfile=<file name>
         When used with 'dump', writes out the records to this file. When used
         with 'createfromdump', loads the records into a new wallet.
    ...

  Commands:
    createfromdump
         Create new wallet file from dumped records

    dump
         Print out all of the wallet key-value records
  ```

  After:

  ```
  Commands:
    createfromdump
         Create new wallet file from dumped records

         -dumpfile=<file name>
              When used with 'dump', writes out the records to this file. When
              used with 'createfromdump', loads the records into a new wallet.

    dump
         Print out all of the wallet key-value records

         -dumpfile=<file name>
              When used with 'dump', writes out the records to this file. When
              used with 'createfromdump', loads the records into a new wallet.
  ```

  Error messages are now generated automatically by `ArgsManager` rather than ad-hoc wallet code for each option:

  Before:
  ```c++
      if (args.IsArgSet("-dumpfile") && command != "dump" && command != "createfromdump") {
          tfm::format(std::cerr, "The -dumpfile option can only be used with the \"dump\" and \"createfromdump\" commands.\n");
          return false;
      }
  ```

  After:
  ```c++
      std::vector<std::string> details;
      if (!args.CheckCommandOptions(command, &details)) {
          tfm::format(std::cerr, "Error: Invalid arguments provided:\n%s\n", util::MakeUnorderedList(details));
          return false;
      }
  ```

  #### Limitations

  - If an option applies to multiple commands, it shares the same help text. There's no way to provide per-command descriptions.
  - Option parsing rules are unchanged — options still cannot appear after the command.

ACKs for top commit:
  achow101:
    ACK 89af67d
  sedited:
    Re-ACK 89af67d
  ryanofsky:
    Code review ACK 89af67d. Since last review: rebase, integration with ClearArgs and fuzz test, and Assume -> Assert switch

Tree-SHA512: 7ae7c3b74d0c8c4db8459e9f0b9c7498b2fa4758954ec49983decbba177877b039779f0f7b55e60c3a0ed74c5e9e4ac4734ba9e049bf3a7743280ef8300869fa
…de_init_tests/init_test

1c500b1 test: avoid non-loopback network traffic from node_init_tests/init_test (Vasil Dimov)

Pull request description:

  The test `node_init_tests/init_test` calls:
  `AppInitMain()` -> `StartMapPort()` -> `StartThreadMapPort()` -> `ThreadMapPort()` -> `ProcessPCP()` -> `PCPRequestPortMap()` -> `CreateSock()` and on the returned value from `CreateSock()` it calls the `Connect()` method.

  Thus, change `BasicTestingSetup::BasicTestingSetup()` to set `-natpmp` to 0. This way `node_init_tests/init_test` or other tests will not do network activity due to `ThreadMapPort()`.

  Also add a comment about `natpmp=0` in
  `test/functional/test_framework/util.py`.

  Also set `-dnsseed=0` in `BasicTestingSetup::BasicTestingSetup()` to
  avoid DNS queries.

ACKs for top commit:
  fjahr:
    re-ACK 1c500b1
  ryanofsky:
    Code review ACK 1c500b1, just disabling -dnsseed since previous review, which makes sense.

Tree-SHA512: 3b275d91361804da6d1dc109dffe741ea4b3dd3be916eb12fa63efa233e13ea4dab5a9f8448bd8bf99bc41817f3b7a768abe91531a3f63eae8b4c912bfcbd13e
Remove redundant int return from btck_chainstate_manager_process_block_header.
Previously returned both an int result and an output validation state parameter, creating ambiguity
where non-zero could mean either invalid header or processing failure. Since ProcessNewBlockHeaders
already provides complete validation info, the int return was redundant.

Co-authored-by: stringintech <[email protected]>
Co-authored-by: stickies-v <[email protected]>
Co-authored-by: Hodlinator <[email protected]>
…connman fuzz harness

371eac8 fuzz: exercise ForNode/ForEachNode callbacks in connman fuzz harness (frankomosh)

Pull request description:

  Track inserted node IDs and sometimes reuse them in `ForNode()` so the successful lookup path is exercised more reliably. Replace no-op callbacks with lightweight CNode accessor calls to make `ForEachNode()` and `ForNode()` cover previously unreached callback code paths.

  This addresses feedback from bitcoin#34830 (comment)  where it was noted that the callbacks had "neither the return type checked nor its side-effect”.

  Coverage reports from the connman fuzz corpus, before and after the change:

  - [Before](https://frankomosh.github.io/fuzz-coverage/connman-callback-coverage/before/index.html)
  - [After](https://frankomosh.github.io/fuzz-coverage/connman-callback-coverage/after/index.html)

  `diff cov_show_before.txt cov_show_after.txt` filtered to `ForNode`/`ForEachNode`/`IsFullOutboundConn`/`ConnectionTypeAsString`:

  **`IsFullOutboundConn` — `net.h:786-788`**
  ```diff
  -  786|      0|    bool IsFullOutboundConn() const {
  -  787|      0|        return m_conn_type == ConnectionType::OUTBOUND_FULL_RELAY;
  -  788|      0|    }
  +  786|  1.13M|    bool IsFullOutboundConn() const {
  +  787|  1.13M|        return m_conn_type == ConnectionType::OUTBOUND_FULL_RELAY;
  +  788|  1.13M|    }
  ```

  **`ConnectionTypeAsString` — `net.h:967`**
  ```diff
  -  967|      0|    std::string ConnectionTypeAsString() const { return ::ConnectionTypeAsString(m_conn_type); }
  +  967|  1.11M|    std::string ConnectionTypeAsString() const { return ::ConnectionTypeAsString(m_conn_type); }
  ```

  **`ForNode` — `net.cpp:4126-4131`**
  ```diff
  -  4126|  1.08k|        if(pnode->GetId() == id) {
  -    |  Branch (4126:12): [True: 0, False: 1.08k]
  -  4127|      0|            found = pnode;
  -  4131|     39|    return found != nullptr && NodeFullyConnected(found) && func(found);
  -                                              ^0                          ^0
  +  4126|    602|        if(pnode->GetId() == id) {
  +    |  Branch (4126:12): [True: 1, False: 601]
  +  4127|      1|            found = pnode;
  +  4131|     28|    return found != nullptr && NodeFullyConnected(found) && func(found);
  +                                              ^1                          ^1
  ```

  **`ForEachNode` — `net.h:1270-1271`**
  ```diff
  -  1270|  1.13M|            if (NodeFullyConnected(node))
  -    |  Branch (1270:17): [True: 0, False: 1.13M]
  -  1271|      0|                func(node);
  +  1270|  1.11M|            if (NodeFullyConnected(node))
  +    |  Branch (1270:17): [True: 1.11M, False: 0]
  +  1271|  1.11M|                func(node);
  ```

  Two previously uncovered functions (`IsFullOutboundConn`, `ConnectionTypeAsString`) are now exercised through the iteration callbacks. `ForNode` finds matching nodes.

ACKs for top commit:
  nervana21:
    tACK 371eac8
  maflcko:
    lgtm ACK 371eac8

Tree-SHA512: 3587c021b16e38ca252676a21b66c5383ab2bd3eec9073e61e9a93db7ef84a94ce5a0c037ac512483680cafabb44103b86df0893e8a9b1bf63b8383bd54f4641
…test

8134857 psbt, test: remove address type restrictions in test (rkrux)

Pull request description:

  Because the corresponding Taproot fields were added in PSBT in bitcoin#22558,
  so these restrictions are no longer necessary.

ACKs for top commit:
  kevkevinpal:
    ACK [8134857](bitcoin@8134857)
  polespinasa:
    lgtm ACK 8134857
  furszy:
    utACK 8134857

Tree-SHA512: 5621509e103674ee5f39454871ca5acb2bf4c16b85dbbdc38abac22fbaea44cafe4bbd91dc3a5c6435b3859a4dc58f12bf7aeec2952c9f80fbbdad2adb52847c
…n CI

75cf970 ci: add one more routable address to the VMs (docker containers) (Vasil Dimov)
1b93983 test: make feature_bind_port_(discover|externalip).py auto-detect the skip condition (Vasil Dimov)

Pull request description:

  `feature_bind_port_discover.py` and `feature_bind_port_externalip.py` require a routable address on the machine to run. Since that was not predictably available on CI, those tests required a manual setting up of IP addresses (e.g. using `ifconfig`) and then running the tests with a command line option telling them that the addresses are set up. The tests were not run in CI and [got rot](bitcoin#31293 (comment)).

  Change that to auto-detect, from the tests, whether the needed IP addresses are present and if yes, run the test, otherwise skip it. Also change the CI to configure the needed addresses when running the functional tests. This way the tests will be run regularly on CI.

  Fixes: bitcoin#31336

ACKs for top commit:
  willcl-ark:
    ACK 75cf970
  frankomosh:
    Tested ACK 75cf970. Built from source.
  ryanofsky:
    Code review ACK 75cf970. Tested locally with and without the special addresses, and the detection seems to work well.

Tree-SHA512: 252911a37a06764f644a1a83c808f5255ac3bc74919426afa5d082c59e1ea924196354735f229d381cb5aff2340e001c2240bbadc8b5f27e5321fb4cfaef0fdb
…btck_BlockValidationState

88d9bc5 kernel: Return btck_BlockValidationState from process_block_header API (yuvicc)

Pull request description:

  This PR refactors `btck_chainstate_manager_process_block_header` to return `btck_BlockValidationState` by value instead of using out-parameters or boolean returns.

ACKs for top commit:
  optout21:
    ACK 88d9bc5
  stickies-v:
    ACK 88d9bc5
  w0xlt:
    reACK 88d9bc5
  hodlinator:
    re-ACK 88d9bc5

Tree-SHA512: f86b6e85aedafd78ae250930cbe34dc666c14d800e43cf8582d49aecb97faab801eff8dcc0250082ceebc3e8d32949839e030cf9f0023b56b23c8f7b7a741e49
Document btck_transaction_check and btck_block_check validation state output parameters as overwritten in-place. This matches their reset-on-entry behavior and avoids implying callers should preserve prior state.
…rwritten in-place

0358c26 kernel: document overwritten validation state outputs (w0xlt)

Pull request description:

  This PR updates the public kernel API documentation for validation-state output parameters that are still caller-provided:

  - `btck_transaction_check`
  - `btck_block_check`

  Both wrappers reset the supplied validation state on entry before running validation, so callers should treat the state as overwritten in-place rather than preserving prior contents.

ACKs for top commit:
  yuvicc:
    re-ACK 0358c26
  sedited:
    ACK 0358c26

Tree-SHA512: f0097c38449c09c6c614a1fb6e5fe09bc84e5dae57c0cb57540419fd6c3f40c06ce8b41e12ab2eff27f4b18d053d32aba2c4a7551a037be93d618b1734922f37
0429c50 bench: Replace Coin Selection bench (Murch)
ec1eefd bench: Remove unnecessary wallet parameter (Murch)
e6c4ffb bench: Fix type mismatch (Murch)

Pull request description:

  Adds a Coin Selection benchmark that doesn’t just test a worst case of one of the algorithms but exercises coin selection to to select inputs for a variety of different targets from a large number of UTXOs.

ACKs for top commit:
  l0rinc:
    code review ACK 0429c50
  sedited:
    ACK 0429c50

Tree-SHA512: 53238d39c8f6d543d80af77e3bb23ab418f2ee266a5ae407fd739c158ca86db553457dcc372b7aa5017f392fb5ae784394cad9edd79b1c0f58ffc32c89e0c306
…ARM intrinsics

86718e4 scripted-diff: rename ABEF_SAVE/CDGH_SAVE to ABCD_SAVE/EFGH_SAVE in SHA-256 ARM intrinsics (jrakibi)

Pull request description:

  ARM SHA256 intrinsics take state in natural order: ABCD + EFGH (hash_abcd/hash_efgh). [Documented here](https://developer.arm.com/architectures/instruction-sets/intrinsics/#f:@navigationhierarchiesinstructiongroup=[Cryptography,SHA256])
  The code already uses that layout, only the `ABEF_SAVE`/`CDGH_SAVE` names were wrong.

  Rename to `ABCD_SAVE`/`EFGH_SAVE`. No logic change.
  Fix in original C code (Jeffrey): noloader/SHA-Intrinsics#14

ACKs for top commit:
  l0rinc:
    ACK 86718e4
  sedited:
    ACK 86718e4

Tree-SHA512: 81c430343f5ee7c9f8d775aa88affb1e3ac60b5df07eac6072d2ba7b53d28c9bb62ea72eee4bc410309f6d38a8abcbb6e27be43dd57d1112b4311e83b58540cf
Adds a small class to parse and query the field lines of an HTTP
response. Used by the upcoming libevent-free HTTPClient implementation.
Re-using the same rpc connection on multiple threads is obviously
unsafe, so this helper can be used to create one connection per thread.

This refactor does not change any behavior and can be reviewed with the
git options:

--color-moved=dimmed-zebra --color-moved-ws=ignore-all-space
rkrux and others added 27 commits June 4, 2026 17:27
The test needs multiple wallets that can be created on a single node, multiple
nodes are not required.

As there is a cost associated with setting-up and tearing-down nodes, this patch
helps in reducing the test time as well.
b2fbd5b ci: run ipc functional tests in arm job (fanquake)

Pull request description:

  These are currently skipped, because `pycapnp` isn't installed (https://github.com/bitcoin/bitcoin/actions/runs/26943765833/job/79499532298#step:10:4286):
  ```bash
  interface_ipc.py                                                                 | ○ Skipped | 0 s
  interface_ipc_mining.py                                                          | ○ Skipped | 0 s
  ```
  They seem to work fine locally. Not sure if this was missed, or on purpose.

ACKs for top commit:
  Sjors:
    ACK b2fbd5b
  sedited:
    ACK b2fbd5b

Tree-SHA512: d9ec06c0d65447102c3354ccddf5c03505e6338a08efd43f6ef495fafba3a6d9bf8c9d8f8e2a29f16931bcc5058911597a08aa938fb40bd9beab8b501c5194ef
`DeserializeHDKeypaths()` was writing into the original `hd_keypaths`
map instead of `deserialized_hd_keypaths`. As a result the latter was
always empty and the round-trip assertion following was trivially true,
so the serialize/deserialize round-trip wasn't actually being exercised.

That bug was introduced with the commit introducing the fuzz target
(commit f898ef6, bitcoin#18994).
…nd-trip

5deb053 fuzz: fix dead HD keypaths (de)serialization round-trip (Sebastian Falbesoner)

Pull request description:

  `DeserializeHDKeypaths()` was writing into the original `hd_keypaths` map instead of `deserialized_hd_keypaths`. As a result the latter was always empty and the round-trip assertion following was trivially true, so the serialize/deserialize round-trip wasn't actually being exercised.

  That bug was introduced with the commit introducing the fuzz target (commit f898ef6, bitcoin#18994).

ACKs for top commit:
  sedited:
    ACK 5deb053

Tree-SHA512: 0d8770aa5da2e132caedd522c8c95c4ceb6d1bcc4d5b6605784fd7d2df41fce29fcd25fc2741c4e751b942b548888833eb9d9d6318505a5c59f7b1f105c990ae
…e node is running without -privatebroadcast set
This is a follow-up to commit faad08e.
Hoisting the NodeClockContext above peer creation ensures m_connected is
captured under mocktime, making the MINIMUM_CONNECT_TIME check
deterministic regardless of which peer is selected for eviction.

This is a prerequisite for the next commit, which removes the
one-second advance from the NodeClockContext default constructor.
The increment was originally added so that mocked time would not appear
to go backward relative to real-clock timestamps captured before
construction, since Now<NodeSeconds>() rounds the current time down to
a whole second. In practice the tests do not mix real and mocked
timestamps in a way that exposes this, so the increment is unnecessary.
This refactor is a follow-up to commit
faad08e and does not
change any behavior.

These call sites are clean mechanical swaps. The remaining ones
require non-trivial test refactoring and are left for future
follow-ups.
The previous name did not indicate the type was intended for
testing. Renaming to FakeNodeClock makes this explicit and
allows call sites to drop the ctx suffix on the variable name.

Suggested in bitcoin#34858 review feedback.

-BEGIN VERIFY SCRIPT-
s() { git grep -l "$1" -- src | xargs sed -i "s/$1/$2/g"; }

s '\<NodeClockContext\>' 'FakeNodeClock'
s '\<clock_ctx\>'        'clock'
-END VERIFY SCRIPT-
SteadyClockContext and FakeNodeClock assume they are the only active
instance. Overlapping them in the same scope would silently clobber
each other.

Add a CRTP base class, LimitOne, that asserts at construction if
another instance already exists.
The cs_LastBlockFile mutex is redundant: all critical sections are
already covered by cs_main. This is demonstrated in this patch by
replacing all instances of locking cs_LastBlockFile with pairs of
`AssertLockHeld(::cs_main)` and `EXCLUSIVE_LOCKS_REQUIRED(::cs_main)`
annotations. No additional `::cs_main` LOCK(...)s are introduced.

It is also not clear for which sections `cs_LastBlockFile` is
responsible for. It is annotated for `m_blockfile_cursors`, but
sporadically and inconsistently also covers `m_blockfile_info`.

Since it has no semantic meaning, and seems confusing to developers,
remove it.
…mutex

ec6cf49 blockstorage: Remove cs_LastBlockFile recursive mutex (sedited)

Pull request description:

  The `cs_LastBlockFile` mutex is redundant: all critical sections are already covered by cs_main. This is demonstrated in this patch by replacing all instances of locking `cs_LastBlockFile` with pairs of `AssertLockHeld(::cs_main)` and `EXCLUSIVE_LOCKS_REQUIRED(::cs_main)` annotations. No additional `::cs_main` LOCK(...)s are introduced (besides for test-only code).

  It is also not clear for which sections `cs_LastBlockFile` is responsible for. It is annotated for `m_blockfile_cursors`, but sporadically and inconsistently also covers `m_blockfile_info` (e.g. in `LoadBlockIndexDB`).

  Since it has no semantic meaning, and seems confusing to developers, remove it.

  An alternative to this patch would be expanding the scope of what `cs_LastBlockFile` covers and turning it into a non-recursive mutex. I prepared such a patch some time ago, but found it unsatisfactory. It was not clear to me if the lock was now covering too much or too little, and its purpose remained unclear. If this patch is accepted, I would expect the project to eventually implement a separate, narrowly-scoped block storage lock to allow for a more parallelizable block processing routine.

ACKs for top commit:
  stickies-v:
    re-ACK ec6cf49
  janb84:
    re- ACK ec6cf49
  pablomartin4btc:
    ACK ec6cf49

Tree-SHA512: e5942bc87300b0db9a0b91d5fe26dab455049e6cef7c96bb12b28141fa04711d46c6af105c0e1a83a9f261edde2c8b8b43ecf577a27d54b4610d784676a85627
The `nb30.h` Windows header defines `UNIQUE_NAME` as a macro.

This introduces a fragile dependency on header inclusion order: if
Windows headers happen to be included before `UNIQUE_NAME` is used, the
preprocessor expands it into a numeric literal, causing syntax errors.

Rename the macro to `BITCOIN_UNIQUE_NAME` to remove this fragility and
avoid the collision entirely.

-BEGIN VERIFY SCRIPT-
sed -i 's/\<UNIQUE_NAME\>/BITCOIN_UNIQUE_NAME/g' $(git grep -l 'UNIQUE_NAME' ./src/)
-END VERIFY SCRIPT-
…tebroadcast fail if privatebroadcast is not enabled

0bfc5e4 add release notes (Pol Espinasa)
fdc9fc1 test: check getprivatebroadcast and abortprivatebroadcast throw if the node is running without -privatebroadcast set (Pol Espinasa)
7b821ef rpc: getprivatebroadcastinfo and abortprivatebroadcast throw if -privatebroadcast is disabled (Pol Espinasa)

Pull request description:

  Makes `getprivatebroadcast` and `abortprivatebroadcast` throw if `-privatebroadcast=0`.

  This is motivated by: sparrowwallet/sparrow#1989

  Knowing if `privatebroadcast` is set can be useful for some external software like Sparrow to avoid call `getprivatebroadcastinfo` each time to see if broadcast was done through that.

ACKs for top commit:
  stickies-v:
    ACK 0bfc5e4
  sedited:
    ACK 0bfc5e4
  rkrux:
    code review ACK 0bfc5e4
  andrewtoth:
    ACK 0bfc5e4

Tree-SHA512: 3bdb3909e93fc3835d801e1efc2bbec673a75a1ff089debd59e8970a0ff2b44d4e00b7ac26f10c972dcb50bf042521921370e1ec57885d67cd8459b3831da898
… to avoid passing 21M BTC

d0b76c7 rpc+bitcoin-tx: Specify correct type for ParseFixedPoint() (Hodlinator)
43ca54c refactor(test): Make CAmount arg explicit for BuildCreditingTransaction() (Hodlinator)
b5e91e9 wallet: Remove CoinsResult::Clear() (Hodlinator)

Pull request description:

  The *knapsack_solver_test* in *coinselector_tests.cpp* was accumulating satoshi amounts beyond 21M BTC. This was uncovered while experimenting with adding checks to `CAmount`. Fix that by fully resetting the `CoinsResult` object accumulating those amounts, inspired by bitcoin#35449 (comment).

  Also, while we're at it, add 2 commits which correct some `int64_t`/`CAmount` confusion.

  Fixes bitcoin#35449

ACKs for top commit:
  sedited:
    ACK d0b76c7
  furszy:
    utACK d0b76c7
  brunoerg:
    code review ACK d0b76c7

Tree-SHA512: 6d989ded6f6327dc657f437dc256d4adf42a34a1252621421ee38d7851c6cdc97a462f033a4728e3aa7d5514deee4db6e83646105633f9cf7ed6e7e90406b67d
…sig_descriptor_psbt

5b65e31 test: remove two unnecessary nodes from the test (rkrux)

Pull request description:

  A discussion in the review of bitcoin#35443 PR brought this test to my attention.

  The test needs multiple wallets that can be created on a single node, multiple nodes are not required.

  As there is a cost associated with setting-up and tearing-down nodes, this patch helps in reducing the test time as well.

ACKs for top commit:
  ekzyis:
    ACK 5b65e31
  polespinasa:
    lgtm ACK 5b65e31
  sedited:
    ACK 5b65e31

Tree-SHA512: f6b4a96b9beee968ef5438fd9db582a48834ff36ba27c19dd7012902528fa713424212530e34cc16b58c19c023f1accd2b89fe846ef2cc36677c24e160c5b817
…QUE_NAME

fba713a scripted-diff: Rename UNIQUE_NAME to BITCOIN_UNIQUE_NAME (Hennadii Stepanov)

Pull request description:

  bitcoin#34454 (comment):
  > ... it is annoying that we keep running into the same bug over and over again (IIRC it happened in the past at least once for Bitcoin Core). Surely this is going to happen again in the future.

  And here we go again.

  ---

  The `nb30.h` Windows header [defines](https://github.com/mingw-w64/mingw-w64/blob/b536c4fdb038a9c59a7e5fb36e7d1293c4dc61d6/mingw-w64-headers/include/nb30.h#L78) `UNIQUE_NAME` as a macro.

  This introduces a fragile dependency on header inclusion order: if Windows headers happen to be included before `UNIQUE_NAME` is used, the preprocessor expands it into a numeric literal, causing syntax errors.

  Rename the macro to `BITCOIN_UNIQUE_NAME` to remove this fragility and avoid the collision entirely.

  ---

  Noticed while doing a Guix build of the [QML repo](https://github.com/bitcoin-core/gui-qml) for Windows.

  Recent similar PRs: bitcoin#34454 and bitcoin#34868.

ACKs for top commit:
  maflcko:
    lgtm ACK fba713a
  sedited:
    ACK fba713a
  w0xlt:
    ACK fba713a

Tree-SHA512: 7a63b99a754e797eb8fa5d6a598606150f47ae1130d1d26067c509830e6575f0378ce63fe0ca35c69dce9a394451a34ddadd8b3d5f6f9a7e4c529108af546fb6
Document why we use LIBCXXABI_USE_LLVM_UNWINDER=OFF.
…adcastToAll

fa2afba p2p: Release m_peer_mutex early in InitiateTxBroadcastToAll (MarcoFalke)

Pull request description:

  The `InitiateTxBroadcastToAll` method holds the `m_peer_mutex` while updating the bloom filters for all peers. This is perfectly fine, because updating the bloom filters is fast. Though, from a style-perspective, the lock does not need to be held for the whole function. Also, holding the lock longer, may confuse Tsan into a lock-order inversion false-positive (ref: bitcoin#19303 (comment)).

  So "fix" both issues in this style-refactor.

ACKs for top commit:
  xyzconstant:
    Code review ACK fa2afba
  shuv-amp:
    ACK fa2afba
  danielabrozzoni:
    Code Review ACK fa2afba
  sedited:
    ACK fa2afba

Tree-SHA512: c47849a4c3cc11c74b61fec3425db8ec7f78db4ca43d7bf3145ce640f7b0872701c09495f0dfe77109d09d5716d920ad3d7308483fe41564c30867b3e80432e7
087f02c ci: skip libunwind runtime in LLVM build (fanquake)
6d47f7c ci: use llvm 22.1.7 (fanquake)

Pull request description:

  Also document why we use `LIBCXXABI_USE_LLVM_UNWINDER=OFF`. Upstream issue is llvm/llvm-project#84348.

ACKs for top commit:
  maflcko:
    lgtm ACK 087f02c
  sedited:
    ACK 087f02c

Tree-SHA512: b93c798fd5a016cad40db9d24cb36cb72e531b284aee5458de41e062960514783e30c6f1413c0e62fa261758d783d0004a0973541cbb36bd34b77800c629bd7a
35a814a test: Limit clocks to one active instance (MarcoFalke)
55e402f scripted-diff: Rename NodeClockContext to FakeNodeClock (seduless)
1e9546f test: Use NodeClockContext in more call sites (seduless)
758fea5 test: Drop ++ from NodeClockContext default constructor (seduless)
7c2ec39 test: Enter mocktime before peer creation in block_relay_only_eviction (seduless)

Pull request description:

  Follow-up to bitcoin#34858

  Updates remaining `SetMockTime` call sites that are clean, mechanical swaps fitting the spirit of the original PR (see: bitcoin#34858 (review) and bitcoin#34858 (comment)). Further updates to `SetMockTime` are more complex and deserve separate, isolated PRs.

  The default constructor for `NodeClockContext` increments to the next tick, which is a defensive measure to prevent time going backwards on construction. This has caused some confusion (see thread: bitcoin#34858 (comment)) and can be safely removed after updating the only test where this is load-bearing (b3c9bd7) (see: bitcoin#34858 (comment)). The removal also tightens the `addrman_tests/addrman_evictionworks` test to sit exactly on the `ADDRMAN_REPLACEMENT` boundary (4h), catching mutations such as:

  ```diff
  diff --git a/src/addrman.cpp b/src/addrman.cpp
  index d3dae59..d0929c62cb 100644
  --- a/src/addrman.cpp
  +++ b/src/addrman.cpp
  @@ -920,3 +920,3 @@ void AddrManImpl::ResolveCollisions_()
                   // Has successfully connected in last X hours
  -                if (current_time - info_old.m_last_success < ADDRMAN_REPLACEMENT) {
  +                if (current_time - info_old.m_last_success <= ADDRMAN_REPLACEMENT) {
                       erase_collision = true;
  ```

  The last follow-up item is updating `NodeClockContext` to `FakeNodeClock` to make it clear it is intended for testing (motivated by bitcoin#34858 (review) and supported in bitcoin#34858 (comment)).

ACKs for top commit:
  maflcko:
    re-ACK 35a814a 🛒
  sedited:
    ACK 35a814a

Tree-SHA512: ade776e288a4b7bbc4c8855c14d61381b5b20329fe1e72fee87f773e47a9519975d58c277fbacda37dd73c0c1d4ce358c92dcdc4ca049d58cb3453ddf751b45b
54de023 guix: add setup.sh (fanquake)

Pull request description:

  This is the first change in bitcoin#25573, which splits out the setup & tarball generation code from `build.sh`, so that it can be re-used, from multiple (future) build scripts.

ACKs for top commit:
  willcl-ark:
    ACK 54de023
  hebasto:
    ACK 54de023.

Tree-SHA512: 9a7f2fe322d281b9867414511af5243f4dd659ea42637f4eb8cc0c8629c94dab842669bb7c503f9fa67cab3fac65561364f07b5c0fda8e6d8c24e7bf161025ef
In ParsePubkeyInner, every extended key required two full base58
decodes: once via DecodeExtKey and again via DecodeExtPubKey, even
though at most one can succeed. Replace both calls with a new
DecodeExtKeyOrPubKey helper that performs a single DecodeBase58Check
and dispatches on the 4-byte version prefix to populate either the
xprv or xpub result.

Also eliminate the per-key std::string allocation in ParsePubkeyInner
by switching the local 'str' variable to std::string_view over the
existing span, and update DecodeSecret / DecodeExtKey / DecodeExtPubKey
to accept std::string_view directly (std::string and const char* callers
are unaffected via implicit conversion).

To allow the string_view path all the way down, the internal base58
decoder is refactored from null-terminated const char* to a (psz, end)
pointer pair, replacing strlen() and *psz null checks with pointer
comparisons. The public DecodeBase58 / DecodeBase58Check APIs now take
std::string_view as well.
@brunoerg brunoerg force-pushed the 2026-04-descriptor branch from 2d86114 to 0d8110a Compare June 10, 2026 15:50
… table

Replace the O(n) linear INPUT_CHARSET.find() in DescriptorChecksum with
a precomputed 256-entry lookup table, reducing the checksum pass from
O(95 * len) to O(len).
@brunoerg brunoerg force-pushed the 2026-04-descriptor branch from 0d8110a to 6dd023a Compare June 10, 2026 17:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.