Skip to content

Conversation

@Sjors
Copy link
Collaborator

@Sjors Sjors commented Aug 29, 2025

Adding coinbase_witness to the Sv2NewTemplate message so that it is automatically propagated to other roles in the Stratum v2 ecosystem, rather than assumed to be 0x00...00.

This ensures that if this value ever gets consensus meaning, miners only need to upgrade their node software, not the rest of the mining stack.

Marked draft because it requires a Stratum v2 spec change as well as support on the SRI side.

There's no rush to do this, since there are currently no soft fork proposals that change the witness reserve value. However, the switch to IPC may be a good opportunity to make a breaking change.

See also:

@plebhash
Copy link
Member

plebhash commented Sep 1, 2025

if I were to write something similar to #15, I'd call this field NewTemplate.coinbase_witness instead of NewTemplate.coinbase_witness_reserved_value

that's because BIP141 says:

The new commitment in coinbase transaction is a hash of the witness root hash and a witness reserved value. The witness reserved value currently has no consensus meaning, but in the future allows new commitment values for future softforks. For example, if a new consensus-critical commitment is required in the future, the commitment in coinbase becomes:

 Double-SHA256(Witness root hash|Hash(new commitment|witness reserved value))

For backward compatibility, the Hash(new commitment|witness reserved value) will go to the coinbase witness, and the witness reserved value will be recorded in another location specified by the future softfork. Any number of new commitment could be added in this way.

Essentially, whoever receives NewTemplate needs to know the contents of the Coinbase witness, which now can be whatever (and it's tipically 0x000...000), and in the future would carry Hash(new commitment|witness reserved value)

In other words, witness reserved value happens to be the in the Coinbase witness today, but it could be somewhere else tomorrow. And what we really want to know is the Coinbase witness so that we can create a Coinbase that will be coherent with the witness commitment.

@Sjors
Copy link
Collaborator Author

Sjors commented Sep 1, 2025

My initial idea was also to call it coinbase_witness, however a witness is a stack of elements and you would expect it to be encoded that way. But the coinbase witness can never have more than one element and that one element MUST be 32 bytes.

That said, I agree that coinbase_witness_reserved_value is also wrong.

Perhaps coinbase_witness_commitment is better. But the word "commitment" is heavily overloaded.

So maybe coinbase_witness is fine, it should be clear enough to how to use it.

@Sjors Sjors force-pushed the 2025/08/witness_reserved_value branch 2 times, most recently from ab3024b to a6ff9da Compare September 1, 2025 17:13
@Sjors Sjors changed the title Sv2NewTemplate: add coinbase_witness_reserved_value Sv2NewTemplate: add coinbase_witness Sep 1, 2025
@Sjors Sjors force-pushed the 2025/08/witness_reserved_value branch from a6ff9da to 29f4071 Compare September 2, 2025 06:36
@Sjors Sjors force-pushed the 2025/08/witness_reserved_value branch from 29f4071 to 979ad78 Compare September 2, 2025 08:35
Adding coinbase_witness to the Sv2NewTemplate message so that it is
automatically propagated to other roles in the Stratum v2 ecosystem,
rather than assumed to be 0x00...00.

This ensures that if this value ever gets consensus meaning, miners
only need to upgrade their node software, not the rest of the mining
stack.

This is a breaking change requiring an updated Stratum v2 spec as
well as support on the SRI side.
@Sjors Sjors force-pushed the 2025/08/witness_reserved_value branch from 979ad78 to 271dd00 Compare October 6, 2025 18:31
@Sjors
Copy link
Collaborator Author

Sjors commented Oct 6, 2025

Rebased after:

Conflict resolution can be reviewed with:

PREV=979ad78b326d16a6e66c2bb1e3e3908538275a97  N=1 && git range-diff `git merge-base --all HEAD $PREV`...$PREV HEAD~$N...HEAD

Sjors added a commit that referenced this pull request Oct 18, 2025
- mark the SHA256 digest returned from Sv2SignatureNoiseMessage::GetHash()
  as initialized so MSan stops flagging the certificate hash
- clear the chain name literals and resulting base params object used by
  Sv2BasicTestingSetup when selecting regtest

This still fails:

==198==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x55b5ca320949 in basic_string /cxx_build/include/c++/v1/string:1003:9
    #1 0x55b5ca320949 in CBaseChainParams /src/sv2-tp/build_fuzz/src/./chainparamsbase.h:29:55
    #2 0x55b5ca320949 in std::__1::unique_ptr<CBaseChainParams, std::__1::default_delete<CBaseChainParams>> std::__1::make_unique[abi:de210103]<CBaseChainParams, char const (&) [8], int, int, 0>(char const (&) [8], int&&, int&&) /cxx_build/include/c++/v1/__memory/unique_ptr.h:759:30
    #3 0x55b5ca31e66f in CreateBaseChainParams(ChainType) /src/sv2-tp/build_fuzz/src/./chainparamsbase.cpp:48:16
    #4 0x55b5ca31dce5 in SelectBaseParams(ChainType) /src/sv2-tp/build_fuzz/src/./chainparamsbase.cpp:55:29
    #5 0x55b5ca2e4cad in Sv2BasicTestingSetup::Sv2BasicTestingSetup() /src/sv2-tp/build_fuzz/src/test/fuzz/./test/sv2_test_setup.cpp:34:5
    #6 0x55b5ca2c381a in std::__1::unique_ptr<Sv2BasicTestingSetup const, std::__1::default_delete<Sv2BasicTestingSetup const>> std::__1::make_unique[abi:de210103]<Sv2BasicTestingSetup const, 0>() /cxx_build/include/c++/v1/__memory/unique_ptr.h:759:30
    #7 0x55b5ca2c339e in (anonymous namespace)::Initialize() /src/sv2-tp/build_fuzz/src/test/fuzz/./test/fuzz/sv2_noise.cpp:32:39
    #8 0x55b5ca2ca01d in __invoke<void (*&)()> /cxx_build/include/c++/v1/__type_traits/invoke.h:87:27
    #9 0x55b5ca2ca01d in __call<void (*&)()> /cxx_build/include/c++/v1/__type_traits/invoke.h:342:5
    #10 0x55b5ca2ca01d in __invoke_r<void, void (*&)()> /cxx_build/include/c++/v1/__type_traits/invoke.h:348:10
    #11 0x55b5ca2ca01d in std::__1::__function::__func<void (*)(), void ()>::operator()() /cxx_build/include/c++/v1/__functional/function.h:174:12
    #12 0x55b5ca2b9622 in operator() /cxx_build/include/c++/v1/__functional/function.h:274:12
    #13 0x55b5ca2b9622 in operator() /cxx_build/include/c++/v1/__functional/function.h:772:10
    #14 0x55b5ca2b9622 in initialize /src/sv2-tp/build_fuzz/src/test/fuzz/./test/fuzz/fuzz.cpp:247:5
    #15 0x55b5ca2b9622 in LLVMFuzzerInitialize /src/sv2-tp/build_fuzz/src/test/fuzz/./test/fuzz/fuzz.cpp:321:5
    #16 0x55b5ca21df42 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:652:5
    #17 0x55b5ca24cfa2 in main /llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
    #18 0x7f0f188e4082 in __libc_start_main (/tmp/not-out/tmp_tv4p4q7/libc.so.6+0x24082) (BuildId: 5792732f783158c66fb4f3756458ca24e46e827d)
    #19 0x55b5ca17802d in _start (/tmp/not-out/tmp_tv4p4q7/sv2_noise_cipher_roundtrip+0x24202d) (BuildId: 4d0ffc8495228d8cab1595a0fea7fa76c40aa18b)
DEDUP_TOKEN: basic_string--CBaseChainParams--std::__1::unique_ptr<CBaseChainParams, std::__1::default_delete<CBaseChainParams>> std::__1::make_unique[abi:de210103]<CBaseChainParams, char const (&) [8], int, int, 0>(char const (&) [8], int&&, int&&)
  Uninitialized value was created by an allocation of 'ref.tmp' in the stack frame
    #0 0x55b5ca32023c in std::__1::unique_ptr<CBaseChainParams, std::__1::default_delete<CBaseChainParams>> std::__1::make_unique[abi:de210103]<CBaseChainParams, char const (&) [8], int, int, 0>(char const (&) [8], int&&, int&&) /cxx_build/include/c++/v1/__memory/unique_ptr.h:759:34
DEDUP_TOKEN: std::__1::unique_ptr<CBaseChainParams, std::__1::default_delete<CBaseChainParams>> std::__1::make_unique[abi:de210103]<CBaseChainParams, char const (&) [8], int, int, 0>(char const (&) [8], int&&, int&&)
SUMMARY: MemorySanitizer: use-of-uninitialized-value /src/sv2-tp/build_fuzz/src/./chainparamsbase.h:29:55 in CBaseChainParams

Assisted-by: GitHub Copilot
Assisted-by: OpenAI GPT-5-Codex
Sjors added a commit that referenced this pull request Oct 21, 2025
- mark the SHA256 digest returned from Sv2SignatureNoiseMessage::GetHash()
  as initialized so MSan stops flagging the certificate hash
- clear the chain name literals and resulting base params object used by
  Sv2BasicTestingSetup when selecting regtest

This still fails:

==198==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x55b5ca320949 in basic_string /cxx_build/include/c++/v1/string:1003:9
    #1 0x55b5ca320949 in CBaseChainParams /src/sv2-tp/build_fuzz/src/./chainparamsbase.h:29:55
    #2 0x55b5ca320949 in std::__1::unique_ptr<CBaseChainParams, std::__1::default_delete<CBaseChainParams>> std::__1::make_unique[abi:de210103]<CBaseChainParams, char const (&) [8], int, int, 0>(char const (&) [8], int&&, int&&) /cxx_build/include/c++/v1/__memory/unique_ptr.h:759:30
    #3 0x55b5ca31e66f in CreateBaseChainParams(ChainType) /src/sv2-tp/build_fuzz/src/./chainparamsbase.cpp:48:16
    #4 0x55b5ca31dce5 in SelectBaseParams(ChainType) /src/sv2-tp/build_fuzz/src/./chainparamsbase.cpp:55:29
    #5 0x55b5ca2e4cad in Sv2BasicTestingSetup::Sv2BasicTestingSetup() /src/sv2-tp/build_fuzz/src/test/fuzz/./test/sv2_test_setup.cpp:34:5
    #6 0x55b5ca2c381a in std::__1::unique_ptr<Sv2BasicTestingSetup const, std::__1::default_delete<Sv2BasicTestingSetup const>> std::__1::make_unique[abi:de210103]<Sv2BasicTestingSetup const, 0>() /cxx_build/include/c++/v1/__memory/unique_ptr.h:759:30
    #7 0x55b5ca2c339e in (anonymous namespace)::Initialize() /src/sv2-tp/build_fuzz/src/test/fuzz/./test/fuzz/sv2_noise.cpp:32:39
    #8 0x55b5ca2ca01d in __invoke<void (*&)()> /cxx_build/include/c++/v1/__type_traits/invoke.h:87:27
    #9 0x55b5ca2ca01d in __call<void (*&)()> /cxx_build/include/c++/v1/__type_traits/invoke.h:342:5
    #10 0x55b5ca2ca01d in __invoke_r<void, void (*&)()> /cxx_build/include/c++/v1/__type_traits/invoke.h:348:10
    #11 0x55b5ca2ca01d in std::__1::__function::__func<void (*)(), void ()>::operator()() /cxx_build/include/c++/v1/__functional/function.h:174:12
    #12 0x55b5ca2b9622 in operator() /cxx_build/include/c++/v1/__functional/function.h:274:12
    #13 0x55b5ca2b9622 in operator() /cxx_build/include/c++/v1/__functional/function.h:772:10
    #14 0x55b5ca2b9622 in initialize /src/sv2-tp/build_fuzz/src/test/fuzz/./test/fuzz/fuzz.cpp:247:5
    #15 0x55b5ca2b9622 in LLVMFuzzerInitialize /src/sv2-tp/build_fuzz/src/test/fuzz/./test/fuzz/fuzz.cpp:321:5
    #16 0x55b5ca21df42 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:652:5
    #17 0x55b5ca24cfa2 in main /llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
    #18 0x7f0f188e4082 in __libc_start_main (/tmp/not-out/tmp_tv4p4q7/libc.so.6+0x24082) (BuildId: 5792732f783158c66fb4f3756458ca24e46e827d)
    #19 0x55b5ca17802d in _start (/tmp/not-out/tmp_tv4p4q7/sv2_noise_cipher_roundtrip+0x24202d) (BuildId: 4d0ffc8495228d8cab1595a0fea7fa76c40aa18b)
DEDUP_TOKEN: basic_string--CBaseChainParams--std::__1::unique_ptr<CBaseChainParams, std::__1::default_delete<CBaseChainParams>> std::__1::make_unique[abi:de210103]<CBaseChainParams, char const (&) [8], int, int, 0>(char const (&) [8], int&&, int&&)
  Uninitialized value was created by an allocation of 'ref.tmp' in the stack frame
    #0 0x55b5ca32023c in std::__1::unique_ptr<CBaseChainParams, std::__1::default_delete<CBaseChainParams>> std::__1::make_unique[abi:de210103]<CBaseChainParams, char const (&) [8], int, int, 0>(char const (&) [8], int&&, int&&) /cxx_build/include/c++/v1/__memory/unique_ptr.h:759:34
DEDUP_TOKEN: std::__1::unique_ptr<CBaseChainParams, std::__1::default_delete<CBaseChainParams>> std::__1::make_unique[abi:de210103]<CBaseChainParams, char const (&) [8], int, int, 0>(char const (&) [8], int&&, int&&)
SUMMARY: MemorySanitizer: use-of-uninitialized-value /src/sv2-tp/build_fuzz/src/./chainparamsbase.h:29:55 in CBaseChainParams

Assisted-by: GitHub Copilot
Assisted-by: OpenAI GPT-5-Codex
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.

3 participants