rpc: fix initialization-order-fiasco by lazy-init of decodepsbt

Crypt-iQ commented at 6:19 PM on April 1, 2026: contributor

Prior to this commit, decodepsbt_inputs would call TxDoc during initialization which lives in another TLU. TxDoc relies on CURRENCY_UNIT to be initialized when it may not have been (note this is different from the TLU containing decodepsbt_inputs which also has a CURRENCY_UNIT). Fix this by lazy initializing decodepsbt_inputs. Also prevent the issue in the future by doing the same for decodepsbt_outputs and getblock_vin.

Curious why the CI missed this, it broke fuzzamoto. It was introduced in fadf901fd4b4d95bd0dd046b8d44a1154c5ea9a8. I was able to trigger this with clang-21 and ASAN_OPTIONS="detect_stack_use_after_return=1:check_initialization_order=1:strict_init_order=1":

=================================================================
==926804==ERROR: AddressSanitizer: initialization-order-fiasco on address 0x5631e5784288 at pc 0x5631e3309f9c bp 0x7ffdb6abc7b0 sp 0x7ffdb6abc7a8
READ of size 8 at 0x5631e5784288 thread T0
    [#0](/bitcoin-bitcoin/0/) 0x5631e3309f9b in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>::size() const /usr/lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/basic_string.h:1064:16
    [#1](/bitcoin-bitcoin/1/) 0x5631e3309f9b in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> std::operator+<char, std::char_traits<char>, std::allocator<char>>(char const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&) /usr/lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/basic_string.tcc:618:35
    [#2](/bitcoin-bitcoin/2/) 0x5631e4238948 in TxDoc(TxDocOptions const&) /root/bitcoin/src/rpc/rawtransaction_util.cpp:382:76
    [#3](/bitcoin-bitcoin/3/) 0x5631e316945e in __cxx_global_var_init.9 /root/bitcoin/src/rpc/rawtransaction.cpp:784:17
    [#4](/bitcoin-bitcoin/4/) 0x5631e3186355 in _GLOBAL__sub_I_rawtransaction.cpp /root/bitcoin/src/rpc/rawtransaction.cpp
    [#5](/bitcoin-bitcoin/5/) 0x7fa6ca046375 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x27375) (BuildId: 79005c16293efa45b441fed45f4f29b138557e9e)
    [#6](/bitcoin-bitcoin/6/) 0x5631e31cf160 in _start (/root/bitcoin/build/bin/bitcoind+0x25b160)

0x5631e5784288 is located 56 bytes before global variable 'CURRENCY_ATOM[abi:cxx11]' defined in '/root/bitcoin/src/policy/feerate.h:20' (0x5631e57842c0) of size 32
  registered at:
    [#0](/bitcoin-bitcoin/0/) 0x5631e31e5738 in __asan_register_globals /root/llvm-project/compiler-rt/lib/asan/asan_globals.cpp:431:3
    [#1](/bitcoin-bitcoin/1/) 0x5631e31e68a9 in __asan_register_elf_globals /root/llvm-project/compiler-rt/lib/asan/asan_globals.cpp:414:3

0x5631e5784288 is located 8 bytes inside of global variable 'CURRENCY_UNIT[abi:cxx11]' defined in '/root/bitcoin/src/policy/feerate.h:19' (0x5631e5784280) of size 32
  registered at:
    [#0](/bitcoin-bitcoin/0/) 0x5631e31e5738 in __asan_register_globals /root/llvm-project/compiler-rt/lib/asan/asan_globals.cpp:431:3
    [#1](/bitcoin-bitcoin/1/) 0x5631e31e68a9 in __asan_register_elf_globals /root/llvm-project/compiler-rt/lib/asan/asan_globals.cpp:414:3

SUMMARY: AddressSanitizer: initialization-order-fiasco /root/bitcoin/src/rpc/rawtransaction_util.cpp:382:76 in TxDoc(TxDocOptions const&)
Shadow bytes around the buggy address:
  0x5631e5784000: f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6
  0x5631e5784080: f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6
  0x5631e5784100: f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6
  0x5631e5784180: f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6
  0x5631e5784200: f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6
=>0x5631e5784280: f6[f6]f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6
  0x5631e5784300: f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6
  0x5631e5784380: f6 f6 f6 f6 01 f9 f9 f9 f6 f6 f6 f6 f6 f6 f6 f6
  0x5631e5784400: f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6
  0x5631e5784480: f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6
  0x5631e5784500: f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==926804==ABORTING

Can be reviewed with git diff HEAD~1 -w since it's mostly indentation.

DrahtBot added the label RPC/REST/ZMQ on Apr 1, 2026

DrahtBot commented at 6:20 PM on April 1, 2026: contributor

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

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	davidgumberg, dergoegge, nervana21
Concept ACK	maflcko, janb84

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#34764 (rpc: replace ELISION references with explicit result fields by satsfy)
#21283 (Implement BIP 370 PSBTv2 by achow101)

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.

davidgumberg commented at 7:03 PM on April 1, 2026: contributor

ACK https://github.com/bitcoin/bitcoin/pull/34988/commits/a09d7913114c1e3345e12e38cf1bc2b0ddc72c8c

Should probably do the same for decodepsbt_outputs.

Note for other reviewers: much smaller diff whitespace changes hidden:

git show a09d791 -b

</summary>

diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp
index 2a1a4d1a06..b226f055ef 100644
--- a/src/rpc/rawtransaction.cpp
+++ b/src/rpc/rawtransaction.cpp
@@ -775,7 +775,9 @@ static RPCMethod signrawtransactionwithkey()
     };
 }

-const RPCResult decodepsbt_inputs{
+const RPCResult& DecodePSBTInputs()
+{
+    static const RPCResult decodepsbt_inputs{
         RPCResult::Type::ARR, "inputs", "",
         {
             {RPCResult::Type::OBJ, "", "",
@@ -930,7 +932,9 @@ const RPCResult decodepsbt_inputs{
                 }},
             }},
         }
-};
+    };
+    return decodepsbt_inputs;
+}

 const RPCResult decodepsbt_outputs{
     RPCResult::Type::ARR, "outputs", "",
@@ -1048,7 +1052,7 @@ static RPCMethod decodepsbt()
                         {
                              {RPCResult::Type::STR_HEX, "key", "(key-value pair) An unknown key-value pair"},
                         }},
-                        decodepsbt_inputs,
+                        DecodePSBTInputs(),
                         decodepsbt_outputs,
                         {RPCResult::Type::STR_AMOUNT, "fee", /*optional=*/true, "The transaction fee paid if all UTXOs slots in the PSBT have been filled."},
                     }

</details>

maflcko commented at 8:09 PM on April 1, 2026: member

I wonder if the workaround commits in https://github.com/bitcoin/bitcoin/pull/25472/commits from 4 years ago are still needed, or if they can be reverted, which should also fix the issue here. cc @hebasto

davidgumberg commented at 10:29 PM on April 1, 2026: contributor

I wonder if the workaround commits in https://github.com/bitcoin/bitcoin/pull/25472/commits from 4 years ago are still needed, or if they can be reverted, which should also fix the issue here. cc @hebasto

Seems kind of stylistically ~50/50 between prior to #24572 and the solution in this PR, maybe not worth the time to test/investigate whether that issue still exists.

maflcko commented at 7:36 AM on April 2, 2026: member

Curious why the CI missed this, it broke fuzzamoto. It was introduced in fadf901. I was able to trigger this with clang-21 and ASAN_OPTIONS="detect_stack_use_after_return=1:check_initialization_order=1:strict_init_order=1":

Do you have the compile options from fuzzamoto, or the steps to reproduce in a fresh container?

I think it would be good to ensure CI can find this.

Seems kind of stylistically ~50/50

Right, at this point it is a style question, but I minimally prefer to have one blob, because the rpc help in the end will also be one blob.

Should probably do the same for decodepsbt_outputs.

Right, the same style should be followed for all three instances. Otherwise, the same issue will later on happen for the other ones.

Concept ACK in any case.

marcofleon commented at 2:41 PM on April 2, 2026: contributor

Curious why the CI missed this, it broke fuzzamoto

The libstdc++ version did it for me. On Debian bookworm with libstdc++-12 it'll crash (all with various clang versions and different linkers). But with libstdc++-14 no crash.

Fuzzamoto uses Debian bookworm while the CI ASAN job uses Ubuntu24.04 which ships with version 14, yes?

Claude helped me out here with this little script:

echo '#include <string>
const std::string x = "BTC";
int main() {}' > /tmp/test_init.cpp

echo "=== GCC 12 ==="
clang++ -std=c++23 -S -o - /tmp/test_init.cpp --gcc-install-dir=/usr/lib/gcc/x86_64-linux-gnu/12 2>&1 | grep -c "cxx_global_var_init"

echo "=== GCC 14 ==="
clang++ -std=c++23 -S -o - /tmp/test_init.cpp --gcc-install-dir=/usr/lib/gcc/x86_64-linux-gnu/14 2>&1 | grep -c "cxx_global_var_init"
=== GCC 12 ===
6
=== GCC 14 ===
0

With GCC 12 headers, the string needs a runtime constructor at startup (the 6 cxx_global_var_init hits there). With GCC 14, it looks like the compiler bakes the string directly into the binary, so ASAN never sees it.

Crypt-iQ commented at 3:29 PM on April 2, 2026: contributor

I would prefer to instead have a revert PR if that's an option because I personally find it cleaner. However, I don't think I could debug the original issue, so I am also happy to fix the other call sites if the investigative effort is not a priority.

Do you have the compile options from fuzzamoto, or the steps to reproduce in a fresh container?

~~I don't have steps to reproduce in a fresh container.~~ I was able to compile on Debian Bookworm:

export ASAN_OPTIONS="detect_stack_use_after_return=1:check_initialization_order=1:strict_init_order=1"; cmake -B build -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ -DSANITIZERS="address"; cmake --build build -j8; ./build/bin/bitcoind

which gives the following configure summary:

<details> <summary>output</summary>


Configure summary
=================
Executables:
  bitcoin ............................. ON
  bitcoind ............................ ON
  bitcoin-node (multiprocess) ......... OFF
  bitcoin-qt (GUI) .................... OFF
  bitcoin-gui (GUI, multiprocess) ..... OFF
  bitcoin-cli ......................... ON
  bitcoin-tx .......................... ON
  bitcoin-util ........................ ON
  bitcoin-wallet ...................... ON
  bitcoin-chainstate (experimental) ... OFF
  libbitcoinkernel (experimental) ..... OFF
  kernel-test (experimental) .......... OFF
Optional features:
  wallet support ...................... ON
  external signer ..................... ON
  ZeroMQ .............................. OFF
  IPC ................................. OFF
  Embedded ASMap ...................... ON
  USDT tracing ........................ OFF
  QR code (GUI) ....................... OFF
  DBus (GUI) .......................... OFF
Tests:
  test_bitcoin ........................ ON
  test_bitcoin-qt ..................... OFF
  bench_bitcoin ....................... OFF
  fuzz binary ......................... OFF

Cross compiling ....................... FALSE
C++ compiler .......................... Clang 21.0.0, /usr/local/llvm-21/bin/clang++
CMAKE_BUILD_TYPE ...................... RelWithDebInfo
Preprocessor defined macros ........... 
C++ compiler flags .................... -O2 -g -std=c++20 -fPIC -fmacro-prefix-map=/root/bitcoin/src=. -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=3 -Wstack-protector -fstack-protector-all -fcf-protection=full -fstack-clash-protection -fsanitize=address -Wall -Wextra -Wgnu -Wcovered-switch-default -Wformat -Wformat-security -Wvla -Wshadow-field -Wthread-safety -Wthread-safety-pointer -Wloop-analysis -Wredundant-decls -Wunused-member-function -Wdate-time -Wconditional-uninitialized -Woverloaded-virtual -Wsuggest-override -Wimplicit-fallthrough -Wunreachable-code -Wdocumentation -Wself-assign -Wundef -Wno-unused-parameter
Linker flags .......................... -O2 -g -fstack-protector-all -fcf-protection=full -fstack-clash-protection -Wl,-z,relro -Wl,-z,now -Wl,-z,separate-code -fsanitize=address -fPIE -pie

NOTE: The summary above may not exactly match the final applied build flags
      if any additional CMAKE_* or environment variables have been modified.
      To see the exact flags applied, build with the --verbose option.

Treat compiler warnings as errors ..... OFF
Use ccache for compiling .............. OFF


-- Configuring done
-- Generating done

</details>

Edit: Claude came up with a one-liner to reproduce in a fresh container:

docker build -t bitcoind - <<'EOF'
FROM debian:bookworm
ENV DEBIAN_FRONTEND=noninteractive
RUN apt-get update && apt-get install -y wget lsb-release software-properties-common gnupg git cmake build-essential libtool autotools-dev automake pkg-config bsdmainutils python3 libevent-dev libboost-dev libsqlite3-dev libzmq3-dev systemtap-sdt-dev libminiupnpc-dev libnatpmp-dev libqrencode-dev
RUN wget https://apt.llvm.org/llvm.sh && chmod +x llvm.sh && ./llvm.sh 21
RUN git clone https://github.com/bitcoin/bitcoin.git /bitcoin
RUN cd /bitcoin && cmake -B build -DCMAKE_C_COMPILER=clang-21 -DCMAKE_CXX_COMPILER=clang++-21 -DBUILD_GUI=OFF -DENABLE_IPC=OFF -DSANITIZERS="address"
RUN cd /bitcoin && cmake --build build -j$(nproc)
RUN cd /bitcoin && ASAN_OPTIONS="detect_stack_use_after_return=1:check_initialization_order=1:strict_init_order=1:abort_on_error=1:handle_abort=1" ./build/bin/bitcoind
EOF

maflcko commented at 4:06 PM on April 2, 2026: member

Claude helped me out here with this little script:

Ah thx. So I guess this is due to constexpr std::string in C++20. So as soon as the heap was used, it'd prevent compile-time evaluation:

/tmp/test_init.cpp:2:29: note: pointer to subobject of heap-allocated object is not a constant expression
    2 | constinit const std::string x = "BTC_LOOOOOOOooooooooooooooooooooooooooooooooooooooooooooooooooonnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnngggggggggggggggggggggggggggggggggggggggggggggggg";
      |                             ^

So I'd presume it would be possible to trigger the failure on recent clang+GCC via this diff:

diff --git a/src/policy/feerate.h b/src/policy/feerate.h
index f6b49a1465..32fd06f09c 100644
--- a/src/policy/feerate.h
+++ b/src/policy/feerate.h
@@ -19,3 +19,3 @@
 const std::string CURRENCY_UNIT = "BTC"; // One formatted unit
-const std::string CURRENCY_ATOM = "sat"; // One indivisible minimum value unit
+const std::string CURRENCY_ATOM = "sat                                                                                                                          "; // One indivisible minimum value unit
 
diff --git a/src/rpc/rawtransaction_util.cpp b/src/rpc/rawtransaction_util.cpp
index d3a1b87580..0aad5e5909 100644
--- a/src/rpc/rawtransaction_util.cpp
+++ b/src/rpc/rawtransaction_util.cpp
@@ -381,3 +381,3 @@ std::vector<RPCResult> TxDoc(const TxDocOptions& opts)
                 {
-                    {RPCResult::Type::STR_AMOUNT, "value", "The value in " + CURRENCY_UNIT},
+                    {RPCResult::Type::STR_AMOUNT, "value", "The value in                                                                                             " + CURRENCY_UNIT},
                     {RPCResult::Type::NUM, "n", "index"},

But that didn't work for me.

Also, I'd presume that adding constinit would remove the asan warning, but that doesn't compile on the GCC-12 stdlib:

# git diff -U1
diff --git a/src/policy/feerate.h b/src/policy/feerate.h
index f6b49a1..07507e9 100644
--- a/src/policy/feerate.h
+++ b/src/policy/feerate.h
@@ -19,3 +19,3 @@
 const std::string CURRENCY_UNIT = "BTC"; // One formatted unit
-const std::string CURRENCY_ATOM = "sat"; // One indivisible minimum value unit
+constinit std::string CURRENCY_ATOM = "sat"; // One indivisible minimum value unit
 
# cmake --build ./bld-cmake -j 1
[  0%] Generating bitcoin-build-info.h
[  0%] Built target generate_build_info
[  1%] Built target bitcoin_clientversion
[  1%] Building CXX object src/wallet/CMakeFiles/bitcoin_wallet.dir/coincontrol.cpp.o
In file included from /b-c/src/wallet/coincontrol.cpp:5:
In file included from /b-c/src/wallet/coincontrol.h:9:
/b-c/src/policy/feerate.h:20:23: error: variable does not have a constant initializer
   20 | constinit std::string CURRENCY_ATOM = "sat"; // One indivisible minimum value unit
      |                       ^               ~~~~~
/b-c/src/policy/feerate.h:20:1: note: required by 'constinit' specifier here
   20 | constinit std::string CURRENCY_ATOM = "sat"; // One indivisible minimum value unit
      | ^~~~~~~~~
/usr/lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/basic_string.h:356:10: note: assignment to member '_M_local_buf' of union with no active member is not allowed in a constant expression
  356 |             __c = _CharT();
      |                 ^

So I guess the only alternative right now would be std::string_view (or just use this pull request as-is).

Debian Bookworm, Clang 21

Thx, I can reproduce with Clang-19 as well in a bookworm container.

Crypt-iQ commented at 9:38 PM on April 2, 2026: contributor

Seems a bit odd that the heap isn't used here, because that'd prevent compile-time evaluation:

Could you expand a little by what you mean here?

Also, I'd presume that adding constinit would remove the asan warning, but that doesn't compile on the GCC-12 stdlib:

I get ODR violations instead with GCC 12.1. Maybe I'm doing something differently?

So I'd presume it would be possible to trigger the failure on recent clang+GCC via this diff:

With GCC-14 I can trigger the failure by modifying CURRENCY_UNIT instead of CURRENCY_ATOM in the diff. I'm guessing that the failure does not occur with small strings because SSO escapes constant evaluation in libstdc++ described here (though they decided the behavior is desirable and not a bug)?

So I guess the only alternative right now would be std::string_view

When I change to std::string_view, I get errors like this:

/root/bitcoin/src/wallet/rpc/coins.cpp:497:201: error: no match for ‘operator+’ (operand types are ‘const char [105]’ and ‘std::string_view’ {aka ‘std::basic_string_view<char>’})
  497 | :Type::STR_AMOUNT, "ancestorfees", /*optional=*/true, "The total fees of in-mempool ancestors (including this one) with fee deltas used for mining priority in " + CURRENCY_ATOM + " (if transaction is in the mempool)"},
      |                                                       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~
      |                                                       |                                                                                                            |
      |                                                       const char [105]                                                                                             std::string_view {aka std::basic_string_view<char>}

maflcko commented at 8:33 AM on April 3, 2026: member

Could you expand a little by what you mean here?

That didn't make sense. Edited my comment.

Also, I'd presume that adding constinit would remove the asan warning, but that doesn't compile on the GCC-12 stdlib:

I get ODR violations instead with GCC 12.1. Maybe I'm doing something differently?

To clarify, I used clang-19 with the GCC-12 stdlib on bookworm. An ODR violation from adding constinit seems odd. To clarify, the diff would look like:

diff --git a/src/policy/feerate.h b/src/policy/feerate.h
index f6b49a1..cdc3b5d 100644
--- a/src/policy/feerate.h
+++ b/src/policy/feerate.h
@@ -19,3 +19,3 @@
 const std::string CURRENCY_UNIT = "BTC"; // One formatted unit
-const std::string CURRENCY_ATOM = "sat"; // One indivisible minimum value unit
+constinit const std::string CURRENCY_ATOM = "sat"; // One indivisible minimum value unit

Maybe you removed the const, which changes the linkage (https://eel.is/c++draft/basic.link#3.2), and then leads to an ODR violation?

In any case, the const constinit std::string suggestion is mostly for fun, because it turns out that the standard does not prescribe a portable SSO. See https://libcxx.llvm.org/UserDocumentation.html#constant-initialized-std-string. So I think it would work in practise for all our needs, but it is probably not something to rely on.

So I guess the only alternative right now would be std::string_view

When I change to std::string_view, I get errors like this:

Yeah, those would need adjustments. I just mentioned it if we want to go down the constinit path in a portable way. It was just a fun idea I had in #22799#issue-979547948 ("Compile the RPC documentation at compile-time to ensure it doesn't change at runtime and is completely static"). However, that'd require a bunch of other changes, so this pull isn't really the right place to take hostage for this discussion.

So I'd presume it would be possible to trigger the failure on recent clang+GCC via this diff:

With GCC-14 I can trigger the failure by modifying CURRENCY_UNIT instead of CURRENCY_ATOM in the diff.

Same here. I don't really understand what is different about CURRENCY_UNIT vs CURRENCY_ATOM to yield this result, though. Edit: My bad, the violating symbol actually is CURRENCY_UNIT. I just got myself confused by glancing at the Asan summary, which mentions CURRENCY_ATOM:

=================================================================
==926804==ERROR: AddressSanitizer: initialization-order-fiasco on address 0x5631e5784288 at pc 0x5631e3309f9c bp 0x7ffdb6abc7b0 sp 0x7ffdb6abc7a8
...

0x5631e5784288 is located 56 bytes before global variable 'CURRENCY_ATOM[abi:cxx11]' defined in '/root/bitcoin/src/policy/feerate.h:20' (0x5631e57842c0) of size 32
...
0x5631e5784288 is located 8 bytes inside of global variable 'CURRENCY_UNIT[abi:cxx11]' defined in '/root/bitcoin/src/policy/feerate.h:19' (0x5631e5784280) of size 32
...

This explains why the CI didn't find it, and I don't think there is anything that can be done to catch UB when the compiler decides to optimize it away. So I think it is clear that there is UB and your patch to fix it lgtm. Let me know if you want it reviewed as-is, or if you want to address any of the style-nits.

maflcko added this to the milestone 31.0 on Apr 3, 2026

maflcko removed this from the milestone 31.0 on Apr 3, 2026

maflcko added this to the milestone 32.0 on Apr 3, 2026

janb84 commented at 12:43 PM on April 3, 2026: contributor

Concept ACK a09d7913114c1e3345e12e38cf1bc2b0ddc72c8c

Think this is the best solution for now, have tried an alternative with constexpr std::string but that does not work (it's a non-transient allocation).

Validated it by running docker containers (could not get the same result on a simple vm):

Validate that it occurs on master:

docker build -t bitcoind - <<'EOF'
FROM debian:bookworm
ENV DEBIAN_FRONTEND=noninteractive
RUN apt-get update && apt-get install -y wget lsb-release software-properties-common gnupg git cmake build-essential libtool autotools-dev automake pkg-config bsdmainutils python3 libevent-dev libboost-dev libsqlite3-dev libzmq3-dev systemtap-sdt-dev libminiupnpc-dev libnatpmp-dev libqrencode-dev
RUN wget https://apt.llvm.org/llvm.sh && chmod +x llvm.sh && ./llvm.sh 21
RUN git clone https://github.com/bitcoin/bitcoin.git /bitcoin
WORKDIR /bitcoin
RUN cmake -B build -DCMAKE_C_COMPILER=clang-21 -DCMAKE_CXX_COMPILER=clang++-21 -DBUILD_GUI=OFF -DENABLE_IPC=OFF -DSANITIZERS="address"
RUN cmake --build build -j$(nproc)
RUN ASAN_OPTIONS="detect_stack_use_after_return=1:check_initialization_order=1:strict_init_order=1:abort_on_error=1:handle_abort=1" ./build/bin/test_bitcoin
EOF

this will error:

 => ERROR [8/8] RUN ASAN_OPTIONS="detect_stack_use_after_return=1:check_initialization_order=1:strict_init_order=1:abort_on_error=1:handle_abort=1" ./build/bin/test_bitcoin                                                                                                                                                             1.8s 
------                                                                                                                                                                                                                                                                                                                                        
 > [8/8] RUN ASAN_OPTIONS="detect_stack_use_after_return=1:check_initialization_order=1:strict_init_order=1:abort_on_error=1:handle_abort=1" ./build/bin/test_bitcoin:                                                                                                                                                                        
0.534 =================================================================                                                                                                                                                                                                                                                                       
0.534 ==7==ERROR: AddressSanitizer: initialization-order-fiasco on address 0x55b95a8b4828 at pc 0x55b95679d13c bp 0x7ffc8b906150 sp 0x7ffc8b906148                                                                                                                                                                                            
0.534 READ of size 8 at 0x55b95a8b4828 thread T0                                                                                                                                                                                                                                                                                              
0.738     [#0](/bitcoin-bitcoin/0/) 0x55b95679d13b in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>::size() const /usr/lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/basic_string.h:1064:16                                                                                                                           
0.738     [#1](/bitcoin-bitcoin/1/) 0x55b95679d13b in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> std::operator+<char, std::char_traits<char>, std::allocator<char>>(char const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&) /usr/lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/basic_string.tcc:618:35
0.738     [#2](/bitcoin-bitcoin/2/) 0x55b958dcba68 in TxDoc(TxDocOptions const&) /bitcoin/src/rpc/rawtransaction_util.cpp:382:76
0.738     [#3](/bitcoin-bitcoin/3/) 0x55b9565e19de in __cxx_global_var_init.9 /bitcoin/src/rpc/rawtransaction.cpp:784:17
0.738     [#4](/bitcoin-bitcoin/4/) 0x55b9565fe8d5 in _GLOBAL__sub_I_rawtransaction.cpp /bitcoin/src/rpc/rawtransaction.cpp
0.738     [#5](/bitcoin-bitcoin/5/) 0x7f12bdb67375 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x27375) (BuildId: 6196744a316dbd57c0fd8968df1680aac482cec4)
0.738     [#6](/bitcoin-bitcoin/6/) 0x55b956644fc0 in _start (/bitcoin/build/bin/test_bitcoin+0x4bffc0) (BuildId: e6458c39400b8bac6eab98c4cbba7c326c561726)
0.738 
1.655 0x55b95a8b4828 is located 56 bytes before global variable 'CURRENCY_ATOM[abi:cxx11]' defined in '/bitcoin/src/policy/feerate.h:20' (0x55b95a8b4860) of size 32
1.655   registered at:
1.655     [#0](/bitcoin-bitcoin/0/) 0x55b95665b6d8 in __asan_register_globals (/bitcoin/build/bin/test_bitcoin+0x4d66d8) (BuildId: e6458c39400b8bac6eab98c4cbba7c326c561726)
1.655     [#1](/bitcoin-bitcoin/1/) 0x55b95665c849 in __asan_register_elf_globals (/bitcoin/build/bin/test_bitcoin+0x4d7849) (BuildId: e6458c39400b8bac6eab98c4cbba7c326c561726)
1.655 
1.656 0x55b95a8b4828 is located 8 bytes inside of global variable 'CURRENCY_UNIT[abi:cxx11]' defined in '/bitcoin/src/policy/feerate.h:19' (0x55b95a8b4820) of size 32
1.656   registered at:
1.656     [#0](/bitcoin-bitcoin/0/) 0x55b95665b6d8 in __asan_register_globals (/bitcoin/build/bin/test_bitcoin+0x4d66d8) (BuildId: e6458c39400b8bac6eab98c4cbba7c326c561726)
1.656     [#1](/bitcoin-bitcoin/1/) 0x55b95665c849 in __asan_register_elf_globals (/bitcoin/build/bin/test_bitcoin+0x4d7849) (BuildId: e6458c39400b8bac6eab98c4cbba7c326c561726)
1.656 
1.656 SUMMARY: AddressSanitizer: initialization-order-fiasco /bitcoin/src/rpc/rawtransaction_util.cpp:382:76 in TxDoc(TxDocOptions const&)
1.656 Shadow bytes around the buggy address:
1.656   0x55b95a8b4580: f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6
1.656   0x55b95a8b4600: f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6
1.656   0x55b95a8b4680: f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6
1.656   0x55b95a8b4700: f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6
1.656   0x55b95a8b4780: f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6
1.656 =>0x55b95a8b4800: f6 f6 f6 f6 f6[f6]f6 f6 f6 f6 f6 f6 f6 f6 f6 f6
1.656   0x55b95a8b4880: f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6
1.656   0x55b95a8b4900: f6 f6 f6 f6 f6 f6 f6 f6 01 f9 f9 f9 f6 f6 f6 f6
1.656   0x55b95a8b4980: f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6
1.656   0x55b95a8b4a00: f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6
1.656   0x55b95a8b4a80: f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6
1.656 Shadow byte legend (one shadow byte represents 8 application bytes):
1.656   Addressable:           00
1.656   Partially addressable: 01 02 03 04 05 06 07 
1.656   Heap left redzone:       fa
1.656   Freed heap region:       fd
1.656   Stack left redzone:      f1
1.656   Stack mid redzone:       f2
1.656   Stack right redzone:     f3
1.656   Stack after return:      f5
1.656   Stack use after scope:   f8
1.656   Global redzone:          f9
1.656   Global init order:       f6
1.656   Poisoned by user:        f7
1.656   Container overflow:      fc
1.656   Array cookie:            ac
1.656   Intra object redzone:    bb
1.656   ASan internal:           fe
1.656   Left alloca redzone:     ca
1.656   Right alloca redzone:    cb
1.656 ==7==ABORTING
1.659 Aborted
------
Dockerfile:9
--------------------
   7 |     RUN cmake -B build -DCMAKE_C_COMPILER=clang-21 -DCMAKE_CXX_COMPILER=clang++-21 -DBUILD_GUI=OFF -DENABLE_IPC=OFF -DSANITIZERS="address"
   8 |     RUN cmake --build build -j$(nproc)
   9 | >>> RUN ASAN_OPTIONS="detect_stack_use_after_return=1:check_initialization_order=1:strict_init_order=1:abort_on_error=1:handle_abort=1" ./build/bin/test_bitcoin
  10 |     
--------------------
ERROR: failed to build: failed to solve: process "/bin/sh -c ASAN_OPTIONS=\"detect_stack_use_after_return=1:check_initialization_order=1:strict_init_order=1:abort_on_error=1:handle_abort=1\" ./build/bin/test_bitcoin" did not complete successfully: exit code: 134

</details>

Validate that is solved by this PR:

docker build -t bitcoind - <<'EOF'
FROM debian:bookworm
ENV DEBIAN_FRONTEND=noninteractive
RUN apt-get update && apt-get install -y wget lsb-release software-properties-common gnupg git cmake build-essential libtool autotools-dev automake pkg-config bsdmainutils python3 libevent-dev libboost-dev libsqlite3-dev libzmq3-dev systemtap-sdt-dev libminiupnpc-dev libnatpmp-dev libqrencode-dev
RUN wget https://apt.llvm.org/llvm.sh && chmod +x llvm.sh && ./llvm.sh 21
RUN git clone https://github.com/bitcoin/bitcoin.git /bitcoin
WORKDIR /bitcoin
RUN git fetch origin pull/34988/head:34988
RUN git switch 34988
RUN cmake -B build -DCMAKE_C_COMPILER=clang-21 -DCMAKE_CXX_COMPILER=clang++-21 -DBUILD_GUI=OFF -DENABLE_IPC=OFF -DSANITIZERS="address"
RUN cmake --build build -j$(nproc)
RUN ASAN_OPTIONS="detect_stack_use_after_return=1:check_initialization_order=1:strict_init_order=1:abort_on_error=1:handle_abort=1" ./build/bin/test_bitcoin
EOF

This completes successfully.

</details>

Crypt-iQ commented at 4:00 PM on April 3, 2026: contributor

Maybe you removed the const, which changes the linkage (https://eel.is/c++draft/basic.link#3.2), and then leads to an ODR violation?

Yup, that was the issue. I think this issue was fixed in GCC-12.3 stdlib (https://gcc.gnu.org/pipermail/gcc-cvs/2022-November/373898.html).

I will push an update that changes decodepbst_outputs as well as getblock_prevout.

rpc: fix initialization-order-fiasco by lazy-init of decodepsbt_inputs

Prior to this commit, decodepsbt_inputs would call TxDoc during initialization
which lives in another TLU. TxDoc relies on CURRENCY_UNIT to be initialized when
it may not have been (note this is different from the TLU containing
decodepsbt_inputs which also has a CURRENCY_UNIT). Fix this by lazy initializing
decodepsbt_inputs.

Prevent the issue from occurring in the future by also doing the same for
decodepsbt_outputs and getblock_vin.

d517fa0a94

Crypt-iQ force-pushed on Apr 3, 2026

davidgumberg commented at 10:26 PM on April 3, 2026: contributor

crACK https://github.com/bitcoin/bitcoin/commit/d517fa0a948ce4d416cd50f27418b76b09d951cb

DrahtBot requested review from maflcko on Apr 3, 2026

DrahtBot requested review from janb84 on Apr 3, 2026

dergoegge approved

dergoegge commented at 9:20 AM on April 7, 2026: member

tACK d517fa0a948ce4d416cd50f27418b76b09d951cb

This fixes the issue for fuzzamoto

nervana21 commented at 6:31 PM on April 7, 2026: contributor

tACK d517fa0a948ce4d416cd50f27418b76b09d951cb

Prior logic led to an initialization-order-fiasco. The undefined behavior was surfaced by fuzzamoto and by compiling with ASAN and enabling initialization-order checking.

This fix is implemented via lazy initialization of decodepsbt_inputs and the related decodepsbt_outputs and getblock_prevout. Other solutions considered include reverting a prior commit or using a different class/type than std::string for the implicated CURRENCY_UNIT global variable.

ryanofsky commented at 7:43 PM on April 7, 2026: contributor

This seems ready to merge, but the idea of fixing this bug by removing these globals and reverting the workaround commits in #25472 seems to have been favored by some reviewers but then forgotten?

I was also confused by the comments starting from #34988 (comment) but I guess they are mostly about attempts to reproduce the bug more reliably and constexpr string masking the bug on some platforms. And then about constinit potentially fixing it but not working with std::string. So I assume these comments are not actionable.

Crypt-iQ commented at 8:03 PM on April 7, 2026: contributor

This seems ready to merge, but the idea of fixing this bug by removing these globals and reverting the workaround commits in #25472 seems to have been favored by some reviewers but then forgotten?

This PR is one alternative -- I am happy to review an alternative PR that reverts the workaround commits. I don't have access to a windows machine and wouldn't be able to reproduce the memory spike issue.

So I assume these comments are not actionable.

Yup these are not actionable.

ryanofsky assigned ryanofsky on Apr 7, 2026

ryanofsky merged this on Apr 7, 2026

ryanofsky closed this on Apr 7, 2026

rpc: fix initialization-order-fiasco by lazy-init of decodepsbt_inputs #34988

Reviews

Conflicts