Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BIP324 ciphersuite #28008

Merged
merged 8 commits into from Aug 10, 2023
Merged

BIP324 ciphersuite #28008

merged 8 commits into from Aug 10, 2023

Conversation

sipa
Copy link
Member

@sipa sipa commented Jun 29, 2023

Depends on #27985 and #27993, based on and partially replaces #25361, part of #27634. Draft while dependencies are not merged.

This adds implementations of:

  • The ChaCha20Poly1305 AEAD from RFC8439 section 2.8, including test vectors.
  • The FSChaCha20 stream cipher as specified in BIP324, a rekeying wrapper around ChaCha20.
  • The FSChaCha20Poly1305 AEAD as specified in BIP324, a rekeying wrapper around ChaCha20Poly1305.
  • A BIP324Cipher class that encapsulates key agreement, key derivation, and stream ciphers and AEADs for BIP324 packet encoding.

The ChaCha20Poly1305 and FSChaCha20Poly1305 implementations are new, taking advance of the improvements in #27993.

@DrahtBot
Copy link
Contributor

DrahtBot commented Jun 29, 2023

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 jamesob, theStack, stratospher

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #28100 (crypto: more Span<std::byte> modernization & follow-ups by sipa)

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.

@sipa sipa mentioned this pull request Jun 29, 2023
43 tasks
@sipa sipa force-pushed the 202306_bip324_ciphersuite branch from 143dd16 to 1b1ae6f Compare June 30, 2023 04:58
@sipa sipa force-pushed the 202306_bip324_ciphersuite branch from 1b1ae6f to ac7483f Compare June 30, 2023 18:06
@sipa sipa force-pushed the 202306_bip324_ciphersuite branch 2 times, most recently from ac43237 to bc9fecf Compare July 7, 2023 18:54
@sipa sipa force-pushed the 202306_bip324_ciphersuite branch 2 times, most recently from 08fe653 to ac6e198 Compare July 7, 2023 21:54
@sipa sipa changed the title BIP324 ciphers: FSChaCha20 and FSChaCha20Poly1305 BIP324 ciphersuite Jul 7, 2023
@sipa sipa force-pushed the 202306_bip324_ciphersuite branch from ac6e198 to 4c1874d Compare July 8, 2023 04:04
@DrahtBot DrahtBot removed the CI failed label Jul 8, 2023
@sipa sipa force-pushed the 202306_bip324_ciphersuite branch 5 times, most recently from 4ff7db7 to 5f9705f Compare July 11, 2023 02:46
@sipa sipa force-pushed the 202306_bip324_ciphersuite branch from 5f9705f to 7714328 Compare July 12, 2023 18:58
@sipa sipa force-pushed the 202306_bip324_ciphersuite branch from 7714328 to d15c57b Compare July 13, 2023 18:09
@sipa sipa marked this pull request as ready for review July 17, 2023 22:43
@sipa sipa force-pushed the 202306_bip324_ciphersuite branch 2 times, most recently from 0b0b07c to b0b6d11 Compare July 17, 2023 22:53
Copy link
Member

@jamesob jamesob left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reACK 1c7582e

Based on the interdiff (https://github.com/jamesob/bitcoin-review-data/blob/master/28008.sipa.bip324_ciphersuite/4.1c7582e/interdiff.3.180909f.diff), which contains the few minor changes @sipa has mentioned above (const declarations, doc adds, small refactors).

by extending my own hacky Python implementation using PyCryptodome (for ChaCha20, ChaCha20Poly1305, SHA256 and HKDF with HMAC_SHA256) + ellswift from our test framework for (see updated gist https://gist.github.com/theStack/e910505e39204c695a073ccc6d63719a).

Nice job @theStack! Less code necessary than I would've thought.

Copy link
Contributor

@theStack theStack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 1c7582e

Finished reviewing. Code looks good to me, checked carefully that the implementation matches BIP324 and verified the test vectors (see above). ✔️

src/test/bip324_tests.cpp Show resolved Hide resolved
@sipa sipa mentioned this pull request Aug 1, 2023
m_chacha20.SetKey32(UCharCast(new_key));
// Wipe the key (a copy remains inside m_chacha20, where it'll be wiped on the next rekey
// or on destruction).
memory_cleanse(new_key, sizeof(new_key));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In 0fee267 "crypto: add FSChaCha20, a rekeying wrapper around ChaCha20"

We have secure_allocator which handles this cleaning when the object is destroyed, could we use that instead of having to remember to explicitly memory_cleanse secret data?

Copy link
Member Author

@sipa sipa Aug 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@achow101 We have a zero_after_free_allocator, which sounds like what you're describing. The secure_allocator goes a step further an allocates from a pool of non-swappable memory pages on supporting systems.

  • The secure_allocator's behavior does sound overkill for the BIP324 purpose, I feel, and it may be counterproductive even because non-swappable memory is limited, and we really want it for our high-quality RNG and wallet keys.
  • The zero_after_free_allocator's behavior is what we want, but actually using an allocator still comes with the additional cost that it would mean FSChaCha20 objects would require an additional heap allocation every time the keys are cycled. I feel that doesn't weigh up against the added ease of understanding that approach brings.

src/test/crypto_tests.cpp Outdated Show resolved Hide resolved
src/crypto/chacha20poly1305.cpp Outdated Show resolved Hide resolved
src/bip324.h Outdated Show resolved Hide resolved
Copy link
Contributor

@stratospher stratospher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tested ACK 1c7582e.

tested it using this branch which uses random inputs from FuzzedDataProvider to check whether the ciphertext produced by this PR and the python BIP code produce the same output.

I also liked the simpler current approach for streaming API changes. the interfaces of the cipher look more efficient and cleaner compared to the previous implementations!

// - Bit 0: whether the ignore bit is set in message
// - Bit 1: whether the responder (0) or initiator (1) sends
// - Bit 2: whether this ciphertext will be corrupted (making it the last sent one)
// - Bit 3-4: controls the maximum aad length (max 511 bytes)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

990f0f8: aad in the BIP allows upto 4095 max bytes (max garbage length). is this a performance saver too?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The (FS)ChaCha20Poly1305 cipher really supports up to $2^{64}-1$ bytes of AAD. It just so happens that we only use it for garbage, and the garbage is limited to 4095 bytes. We need some kind of limit anyway, but it's a fair point that for our purposes, 4095 would be a more natural limit.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done in #28267.

src/test/bip324_tests.cpp Show resolved Hide resolved
@fanquake
Copy link
Member

We can track and deal with all remaining comments in one of the followup PRs.

@fanquake fanquake merged commit b2ec032 into bitcoin:master Aug 10, 2023
15 checks passed
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Aug 10, 2023
stratospher added a commit to stratospher/bitcoin that referenced this pull request Aug 13, 2023
* move `dummy_tag` variable in FSChaCha20Poly1305 crypto_tests
outside of the loop to be reused every time
* use easy to read `cipher.last()` in `AEADChaCha20Poly1305::Decrypt()`
* comment for initiator in `BIP324Cipher::Initialize()`
@stratospher
Copy link
Contributor

stratospher commented Aug 13, 2023

we'd want fuzz tests for AEADChaCha20Poly1305 and FSChaCha20Poly1305 right?
wrote one in #28263 and picked up some review suggestions from here in #28267 in case it helps.

stratospher added a commit to stratospher/bitcoin that referenced this pull request Aug 14, 2023
follow-up to bitcoin#28008.
* move `dummy_tag` variable in FSChaCha20Poly1305 crypto_tests
outside of the loop to be reused every time
* use easy to read `cipher.last()` in `AEADChaCha20Poly1305::Decrypt()`
* comment for initiator in `BIP324Cipher::Initialize()`
* systematically damage ciphertext with bit positions in bip324_tests
* use 4095 max bytes for aad in bip324 fuzz test
fanquake added a commit that referenced this pull request Aug 15, 2023
93cb8f0 refactor: add missing headers for BIP324 ciphersuite (stratospher)
d22d5d9 crypto: BIP324 ciphersuite follow-up (stratospher)

Pull request description:

  follow-up to #28008.
  * move `dummy_tag` variable in FSChaCha20Poly1305 crypto_tests outside of the loop to be reused every time
  * use easy to read `cipher.last()` in `AEADChaCha20Poly1305::Decrypt()`
  * comment for initiator in `BIP324Cipher::Initialize()`
  * systematically damage ciphertext with bit positions in bip324_tests
  * use 4095 max bytes for `aad` in bip324 fuzz test

ACKs for top commit:
  fanquake:
    ACK 93cb8f0 - thanks for following up here.

Tree-SHA512: 361f3e226d3168fdef69a2eebe6092cfc04ba14ce009420222e762698001eaf8be69a1138dab0be237964509c2b96a41a0b4db5c1df43ef75062f143c5aa741a
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Aug 17, 2023
93cb8f0 refactor: add missing headers for BIP324 ciphersuite (stratospher)
d22d5d9 crypto: BIP324 ciphersuite follow-up (stratospher)

Pull request description:

  follow-up to bitcoin#28008.
  * move `dummy_tag` variable in FSChaCha20Poly1305 crypto_tests outside of the loop to be reused every time
  * use easy to read `cipher.last()` in `AEADChaCha20Poly1305::Decrypt()`
  * comment for initiator in `BIP324Cipher::Initialize()`
  * systematically damage ciphertext with bit positions in bip324_tests
  * use 4095 max bytes for `aad` in bip324 fuzz test

ACKs for top commit:
  fanquake:
    ACK 93cb8f0 - thanks for following up here.

Tree-SHA512: 361f3e226d3168fdef69a2eebe6092cfc04ba14ce009420222e762698001eaf8be69a1138dab0be237964509c2b96a41a0b4db5c1df43ef75062f143c5aa741a
Retropex pushed a commit to Retropex/bitcoin that referenced this pull request Oct 4, 2023
follow-up to bitcoin#28008.
* move `dummy_tag` variable in FSChaCha20Poly1305 crypto_tests
outside of the loop to be reused every time
* use easy to read `cipher.last()` in `AEADChaCha20Poly1305::Decrypt()`
* comment for initiator in `BIP324Cipher::Initialize()`
* systematically damage ciphertext with bit positions in bip324_tests
* use 4095 max bytes for aad in bip324 fuzz test
Retropex added a commit to Retropex/bitcoin that referenced this pull request Oct 4, 2023
Release note for changing -permitbaremultisig config default to false

Remove unused MessageStartChars parameters from BlockManager methods

tracepoints: Disables `-Wgnu-zero-variadic-macro-arguments` to compile without warnings

Fixes #26916 by disabling the warning `-Wgnu-zero-variadic-macro-arguments` when clang is used as the compiler.

lint: drop DIR_IWYU global

lint: remove lint-logs.py

lint: remove  /* Continued */ markers from codebase

refactor: fix unterminated LogPrintf()s

tidy: Integrate bicoin-tidy clang-tidy plugin

Enable `bitcoin-unterminated-logprintf`.

Co-authored-by: Cory Fields <cory-nospam-@coryfields.com>

doc: remove Fedora libdb4-*-devel install docs

These are no-longer installable on any recent Fedora (33+).
Remove the install instructions.
Fix the typo in the Ubuntu/Debian instructions.

refactor: Wrap DestroyDB in dbwrapper helper

Wrap leveldb::DestroyDB in a helper function without exposing
leveldb-specifics.

Also, add missing optional include.

The context of this commit is an effort to decouple the dbwrapper header
file from leveldb includes. To this end, the includes are moved to the
dbwrapper implementation file. This is done as part of the kernel
project to reduce the number of required includes for users of the
kernel.

refactor: Split dbwrapper CDBBatch::Write implementation

Keep the generic serialization in the header, while moving
leveldb-specifics to the implementation file.

The context of this commit is an effort to decouple the dbwrapper header
file from leveldb includes. To this end, the includes are moved to the
dbwrapper implementation file. This is done as part of the kernel
project to reduce the number of required includes for users of the
kernel.

refactor: Split dbwrapper CDBatch::Erase implementation

Keep the generic serialization in the header, while moving
leveldb-specifics to the implementation file.

The context of this commit is an effort to decouple the dbwrapper header
file from leveldb includes. To this end, the includes are moved to the
dbwrapper implementation file. This is done as part of the kernel
project to reduce the number of required includes for users of the
kernel.

refactor: Pimpl leveldb::batch for CDBBatch

Hide the leveldb::WriteBatch member variable with a pimpl in order not
to expose it directly in the header.

Also move CDBBatch::Clear to the dbwrapper implementation to use the new
impl_batch.

The context of this commit is an effort to decouple the dbwrapper header
file from leveldb includes. To this end, the includes are moved to the
dbwrapper implementation file. This is done as part of the kernel
project to reduce the number of required includes for users of the
kernel.

refactor: Split dbwrapper CDBIterator::Seek implementation

Keep the generic serialization in the header, while moving
leveldb-specifics to the implementation file.

The context of this commit is an effort to decouple the dbwrapper header
file from leveldb includes. To this end, the includes are moved to the
dbwrapper implementation file. This is done as part of the kernel
project to reduce the number of required includes for users of the
kernel.

refactor: Split dbwrapper CDBIterator::GetKey implementation

Keep the generic serialization in the header, while moving
leveldb-specifics to the implementation file.

The context of this commit is an effort to decouple the dbwrapper header
file from leveldb includes. To this end, the includes are moved to the
dbwrapper implementation file. This is done as part of the kernel
project to reduce the number of required includes for users of the
kernel.

refactor: Split dbwrapper CDBIterator::GetValue implementation

Keep the generic serialization in the header, while moving
leveldb-specifics to the implementation file.

The context of this commit is an effort to decouple the dbwrapper header
file from leveldb includes. To this end, the includes are moved to the
dbwrapper implementation file. This is done as part of the kernel
project to reduce the number of required includes for users of the
kernel.

refactor: Pimpl leveldb::Iterator for CDBIterator

Hide the leveldb::Iterator member variable with a pimpl in order not to
expose it directly in the header.

Also, move CDBWrapper::NewIterator to the dbwrapper implementation to
use the pimpl for CDBIterator initialziation.

The context of this commit is an effort to decouple the dbwrapper header
file from leveldb includes. To this end, the includes are moved to the
dbwrapper implementation file. This is done as part of the kernel
project to reduce the number of required includes for users of the
kernel.

refactor: Split dbwrapper CDBWrapper::Read implementation

Keep the generic serialization in the header, while moving
leveldb-specifics to the implementation file.

The context of this commit is an effort to decouple the dbwrapper header
file from leveldb includes. To this end, the includes are moved to the
dbwrapper implementation file. This is done as part of the kernel
project to reduce the number of required includes for users of the
kernel.

refactor: Fix logging.h includes

These were uncovered as missing by the next commit.

refactor: Split dbwrapper CDBWrapper::Exists implementation

Keep the generic serialization in the header, while moving
leveldb-specifics to the implementation file.

The context of this commit is an effort to decouple the dbwrapper header
file from leveldb includes. To this end, the includes are moved to the
dbwrapper implementation file. This is done as part of the kernel
project to reduce the number of required includes for users of the
kernel.

refactor: Move HandleError to dbwrapper implementation

Make it a static function in dbwrapper.cpp, since it is not used
elsewhere and when left in the header, would expose a leveldb type.

The context of this commit is an effort to decouple the dbwrapper header
file from leveldb includes. To this end, the includes are moved to the
dbwrapper implementation file. This is done as part of the kernel
project to reduce the number of required includes for users of the
kernel.

refactor: Split dbwrapper CDBWrapper::EstimateSize implementation

Keep the generic serialization in the header, while moving
leveldb-specifics to the implementation file.

Since CharCast is no longer needed in the header, move it to the
implementation file.

The context of this commit is an effort to decouple the dbwrapper header
file from leveldb includes. To this end, the includes are moved to the
dbwrapper implementation file. This is done as part of the kernel
project to reduce the number of required includes for users of the
kernel.

refactor: Move CDBWrapper leveldb members to their own context struct

The context of this commit is an effort to decouple the dbwrapper header
file from leveldb includes. To this end, the includes are moved to the
dbwrapper implementation file. This is done as part of the kernel
project to reduce the number of required includes for users of the
kernel.

build: Remove leveldb from BITCOIN_INCLUDES

Since leveldb is no longer in our header tree, move its include flags to
whereever dbwrapper.cpp is built.

refactor: Correct dbwrapper key naming

The ss- prefix should connotate a DataStream variable. Now that these
variables are byte spans, drop the prefix.

ci: Use qemu-user through container engine

doc: document PeerManager::Options members

net processing: clamp -maxorphantx to uint32_t bounds

net processing: clamp -blockreconstructionextratxn to uint32_t bounds

Also changes max_extra_txs into a uint32_t to avoid platform-specific
behaviour

doc: Add release note

crypto: remove outdated variant of ChaCha20Poly1305 AEAD

Remove the variant of ChaCha20Poly1305 AEAD that was previously added in
anticipation of BIP324 using it. BIP324 was updated to instead use rekeying
wrappers around otherwise unmodified versions of the ChaCha20 stream cipher
and the ChaCha20Poly1305 AEAD as specified in RFC8439.

crypto: add the ChaCha20Poly1305 AEAD as specified in RFC8439

This adds an implementation of the ChaCha20Poly1305 AEAD exactly matching
the version specified in RFC8439 section 2.8, including tests and official
test vectors.

crypto: add FSChaCha20, a rekeying wrapper around ChaCha20

This adds the FSChaCha20 stream cipher as specified in BIP324, a
wrapper around the ChaCha20 stream cipher (specified in RFC8439
section 2.4) which automatically rekeys every N messages, and
manages the nonces used for encryption.

Co-authored-by: dhruv <856960+dhruv@users.noreply.github.com>

crypto: add FSChaCha20Poly1305, rekeying wrapper around ChaCha20Poly1305

This adds the FSChaCha20Poly1305 AEAD as specified in BIP324, a wrapper
around the ChaCha20Poly1305 AEAD (as specified in RFC8439 section 2.8) which
automatically rekeys every N messages, and automatically increments the nonce
every message.

bench: add benchmark for FSChaCha20Poly1305

Add a benchmark for FSChaCha20Poly1305 encryption, so the overhead of key
generation and authentication can be observed for various message sizes.

crypto: support split plaintext in ChaCha20Poly1305 Encrypt/Decrypt

Add BIP324Cipher, encapsulating key agreement, derivation, and stream/AEAD ciphers

Co-authored-by: dhruv <856960+dhruv@users.noreply.github.com>

tests: add decryption test to bip324_tests

doc: use llvm-config for bitcoin-tidy example

An LLVM installation will have `llvm-config` available to query for
info. Ask it for the `--cmakedir`, and use that in our bitcoin-tidy
example, rather than listing multiple different (potential) paths per
distro/OS etc.

bitcoin-tidy: fix macOS build

LLVM uses these options for building as well, so there's precedent.

Also fix the shared library extension which was incorrectly being set to dylib

test: locked_wallet, skip default fee estimation

Same as we do with the nodes default wallets.
No test case on this file is meant to exercise fee estimation.

fuzz: coins_view: correct an incorrect assertion

It is incorrect to assert that `cache.HaveCoin()` will always be `true`
if `backend.HaveCoin()` is. The coin could well have been marked as
spent in the cache but not yet flushed, in which case `cache.HaveCoin()`
would return `false`.

Note this was never hit because `exists_using_have_coin_in_backend` is
currently never `true` (it's the default implementation of `CCoinsView`.
However this might change if we were to add a target where the backend
is a `CCoinsViewDB`.

fuzz: coins_view: remove an incorrect assertion

Again, this was not hit because the default implementation of
`CCoinsView` return `false` for `GetCoin`.

crypto: BIP324 ciphersuite follow-up

follow-up to #28008.
* move `dummy_tag` variable in FSChaCha20Poly1305 crypto_tests
outside of the loop to be reused every time
* use easy to read `cipher.last()` in `AEADChaCha20Poly1305::Decrypt()`
* comment for initiator in `BIP324Cipher::Initialize()`
* systematically damage ciphertext with bit positions in bip324_tests
* use 4095 max bytes for aad in bip324 fuzz test

refactor: add missing headers for BIP324 ciphersuite

ci: Drop no longer needed `macos_sdk_cache`

It has been cached in the Docker image since https://github.com/bitcoin/bitcoin/pull/27028.

ci: Drop BASE_SCRATCH_DIR from LIBCXX_DIR

Using a hard-coded path avoids non-determinism issues and improves CI
UX.

ci: Only create folders when needed

Now that container volumes are used, the folders are no longer mounted.
They are only needed when running without a container engine (docker,
podman).

ci: Use hard-coded root path for CI containers

ci: Run "macOS native x86_64" job on GitHub Actions

Also, the "macOS native arm64" task has been removed from Cirrus CI.

Remove unused boost signals2 from torcontrol

iwyu on torcontrol

Replace LocaleIndependentAtoi with ToIntegral

No need for saturating behavior when the int is composed of 3 digits.

remove unused limits.h include in compat.h

Sort includes in compat.h

Can be reviewed with:
--color-moved=blocks  --color-moved-ws=ignore-all-space --ignore-all-space

test: check backup from `migratewallet` can be successfully restored

ci: Fix macOS-cross SDK rsync

This should fix the macOS-cross build on Cirrus CI containers.

Locally this was already working, because the SDK was cached in
/ci_container_base/ in the image, which is also the folder used for a
later CI run.

However, on Cirrus CI, when using an image *and* a custom BASE_ROOT_DIR,
the SDK will not be found in /ci_base_install/, nor in BASE_ROOT_DIR.

Fix this by normalizing *all* folders to /ci_container_base/.

ci: Avoid error on macOS native

This avoids "mkdir: /ci_container_base: Read-only file system"

mempool_entry: add mempool entry sequence number

validation: when adding txs due to a block reorg, allow immediate relay

net_processing: drop m_recently_announced_invs bloom filter

Rather than using a bloom filter to track announced invs, simply allow
a peer to request any tx that entered the mempool prior to the last INV
message we sent them. This also obsoletes the UNCONDITIONAL_RELAY_DELAY.

net_processing: don't add txids to m_tx_inventory_known_filter

We no longer have m_recently_announced_invs, so there is no need to add
txids to m_tx_inventory_known_filter to dedupe that filter.

test: Check tx from disconnected block is immediately requestable

Check that peers can immediately request txs from blocks that have been
reorged out and are now in our mempool.

net_processing: Clean up INVENTORY_BROADCAST_MAX constants

mempool_entry: improve struct packing

ci: Move tidy to persistent worker

refactor: Remove PERSISTENT_WORKER_* yaml templates

* PERSISTENT_WORKER_TEMPLATE_ENV is not needed at all, because
  RESTART_CI_DOCKER_BEFORE_RUN is already set on the persistent worker.
* PERSISTENT_WORKER_TEMPLATE can be replaced by pinning the
  previous_releases task to a type of worker. This should make the CI
  performance more consistent.

Move MANDATORY_SCRIPT_VERIFY_FLAGS from script/standard.h to policy/policy.h

Remove ScriptHash from CScriptID constructor

Replaces the constructor in CScriptID that converts a ScriptHash with a
function ToScriptID that does the same. This prepares for a move of
CScriptID to avoid a circular dependency.

Move CScriptID to script.{h/cpp}

CScriptID should be next to CScript just as CKeyID is next to CPubKey

Move Taproot{SpendData/Builder} to signingprovider.{h/cpp}

TaprootSpendData and TaprootBuilder are used in signing in
SigningProvider contexts, so they should live near that.

Move CTxDestination to its own file

CTxDestination is really our internal representation of an address and
doesn't really have anything to do with standard script types, so move
them to their own file.

MOVEONLY: Move datacarrier defaults to policy.h

Clean up things that include script/standard.h

Remove standard.h from files that don't use anything in it, and include
it in files that do.

Clean up script/standard.{h/cpp} includes

Rename script/standard.{cpp/h} to script/solver.{cpp/h}

Since script/standard only contains things that are used by the Solver
and its callers, rename the files to script/solver.

Rework receive buffer pushback

Co-authored-by: Anthony Towns <aj@erisian.com.au>

test: Fix intermittent issue in mining_getblocktemplate_longpoll.py

Bugfix: RPC: Remove quotes from non-string oneline descriptions

RPC/rpcdoccheck: Error if a oneline_description has a quote for a non-string

RPC/Mining: Document template_request better for getblocktemplate

ci: Ensure that only a single workflow processes `github.ref` at a time

ci: Refactor: Remove CI_USE_APT_INSTALL

rpc: remove one more quote from non-string oneline description

This fixes a silent conflict betwen #28123 and #27460

crypto: refactor ChaCha20 classes to use Span<std::byte> interface

random: simplify FastRandomContext::randbytes using fillrand

crypto: require key on ChaCha20 initialization

fuzz: support std::byte in Consume{Fixed,Variable}LengthByteVector

tests: miscellaneous hex / std::byte improvements

crypto: make ChaCha20::SetKey wipe buffer

doc: Fix bitcoin-unterminated-logprintf tidy comments

* Move module description from test to LogPrintfCheck
* Add test doc
* Remove unused comment, see https://github.com/bitcoin/bitcoin/pull/26296/files#r1279351539

refactor: Enforce C-str fmt strings in WalletLogPrintf()

refactor: Enable all clang-tidy plugin bitcoin tests

This makes it easier to add new ones without having to modify this file
every time.

bitcoin-tidy: Apply bitcoin-unterminated-logprintf to spkm as well

ci: Add missing ${CI_RETRY_EXE} before curl

ci: Add missing amd64 to win64-cross task

Also, do the same for android, which also fails.

assumeutxo cleanup: Move IsInitialBlockDownload & NotifyHeaderTip to ChainstateManager

This change makes IsInitialBlockDownload and NotifyHeaderTip functions no
longer tied to individual Chainstate objects. It makes them work with the
ChainstateManager object instead so code is simpler and it is no longer
possible to call them incorrectly with an inactive Chainstate.

This change also makes m_cached_finished_ibd caching easier to reason about,
because now there is only one cached value instead of two (for background and
snapshot chainstates) so the cached IBD state now no longer gets reset when a
snapshot is loaded.

There should be no change in behavior because these functions were always
called on the active ChainState objects.

These changes were discussed previously
https://github.com/bitcoin/bitcoin/pull/27746#discussion_r1246868905 and
https://github.com/bitcoin/bitcoin/pull/27746#discussion_r1237552792 as
possible followups for that PR.

ci: Disable cache save for pull requests in GitHub Actions

Otherwise, multiple pull requests fill GitHub Actions cache quota
shortly.

tx fees, policy: doc: update and delete unnecessary comment

test: ensure acceptstalefeeestimates is supported only on regtest chain

test: fix 'unknown named parameter' test in `wallet_basic`

Fixes loop when testing an unknown named parameter.

Remove unused includes from txmempool.h

... and move them to where they are really needed.

This was found by IWYU:

txmempool.h should remove these lines:
- #include <random.h>  // lines 29-29
- class CBlockIndex;  // lines 43-43
- class Chainstate;  // lines 45-45

Also, move the stdlib section to the right place. Can be reviewed with:
--color-moved=dimmed-zebra

move-only: Create src/kernel/mempool_removal_reason.h

This is needed for a future commit. Can be reviewed with:
--color-moved=dimmed-zebra

Remove unused includes from blockfilter.h

This removes unused includes, primitives/block found manually, and the
others by iwyu:

blockfilter.h should remove these lines:
- #include <serialize.h>  // lines 16-16
- #include <undo.h>  // lines 18-18

Remove unused includes from wallet.cpp

This removes unused includes, such as undo.h or txmempool.h from
wallet.cpp.

Also, add missing ones, according to IWYU.

test: Support riscv64 in get_previous_releases.py

refactor: Add missing includes

Refactor: Remove confusing static_cast

guix: pre time-machine bump changes (Windows)

Split out of #27897. This is some refactoring to the Windows Guix build
that facilitates bumping our Guix time-machine. Namely, avoiding
`package-with-extra-configure-variable`, which is non-functional in the
newer time-machine, see https://issues.guix.gnu.org/64436.

At the same time, consolidate our Windows GCC build into mingw-w64-base-gcc.
Rename `gcc-10-remap-guix-store.patch` to avoid changing it whenever GCC changes.

We move the old `building-on` inside `explicit-cross-configure`, so that
non-windows builds continue to work. Note that `explicit-cross-configure`
will be going away entirely (see #27897).

[test framework] make it easier to fast-forward setmocktime

Have each TestNode keep track of the last timestamp it called
setmocktime with, and add a bumpmocktime() function to bump by a
number of seconds. Makes it easy to fast forward n seconds without
keeping track of what the last timestamp was.

[functional test] transaction orphan handling

ci: Switch remaining tasks to self-hosted

This allows to drop unused templates, such as
cirrus_ephemeral_worker_template_env, or container_depends_template.

Also, ccache_cache, previous_releases_cache, and
base_depends_built_cache can be dropped, because the caching is done in
container volumes on the self-hosted runners.

ci: Remove distro-name from task name

The exact distro name should not be important. Also, it is easy to find
out, if needed. Thus, remove it to avoid bloat and maintenance overhead
having to keep it in sync.

doc, policy: Clarify comment on STANDARD_SCRIPT_VERIFY_FLAGS

Make post-p2sh consensus rules mandatory for tx relay

Update help text for spend and rawtransaction rpcs

fixing typo

guix: consolidate glibc 2.27 package

Refactor our glibc 2.27 to be a single 'package', and avoid the use of
`package-with-extra-configure-variable`. This also lets us drop the
`enable_werror` workaround, and just use --disable-werror directly.

Employ the same workaround as the Guix glibc, to avoid a "permission
denied" failure during build:
```bash
make  subdir=sunrpc -C sunrpc ..=../ subdir_install
make[2]: Entering directory '/tmp/guix-build-glibc-cross-x86_64-linux-gnu-2.27.drv-0/source/sunrpc'
.././scripts/mkinstalldirs /gnu/store/ga8jciqrd5lh52m572x3mk4q1smf5agq-glibc-cross-x86_64-linux-gnu-2.27/include/rpc
mkdir -p -- /gnu/store/ga8jciqrd5lh52m572x3mk4q1smf5agq-glibc-cross-x86_64-linux-gnu-2.27/include/rpc
/gnu/store/kvpvk5wh70wdbjnr83hh85rg22ysxm9h-coreutils-8.32/bin/install -c -m 644 rpc/netdb.h /gnu/store/ga8jciqrd5lh52m572x3mk4q1smf5agq-glibc-cross-x86_64-linux-gnu-2.27/include/rpc/netdb.h
.././scripts/mkinstalldirs /gnu/store/ga8jciqrd5lh52m572x3mk4q1smf5agq-glibc-cross-x86_64-linux-gnu-2.27/include/nfs
mkdir -p -- /gnu/store/ga8jciqrd5lh52m572x3mk4q1smf5agq-glibc-cross-x86_64-linux-gnu-2.27/include/nfs
/gnu/store/kvpvk5wh70wdbjnr83hh85rg22ysxm9h-coreutils-8.32/bin/install -c -m 644 ../sysdeps/unix/sysv/linux/nfs/nfs.h /gnu/store/ga8jciqrd5lh52m572x3mk4q1smf5agq-glibc-cross-x86_64-linux-gnu-2.27/include/nfs/nfs.h
/gnu/store/kvpvk5wh70wdbjnr83hh85rg22ysxm9h-coreutils-8.32/bin/install -c -m 644 /tmp/guix-build-glibc-cross-x86_64-linux-gnu-2.27.drv-0/build/gnu/lib-names-64.h /gnu/store/ga8jciqrd5lh52m572x3mk4q1smf5agq-glibc-cross-x86_64-linux-gnu-2.27/include/gnu/lib-names-64.h
/gnu/store/kvpvk5wh70wdbjnr83hh85rg22ysxm9h-coreutils-8.32/bin/install -c -m 644 etc.rpc /etc/rpc
/gnu/store/kvpvk5wh70wdbjnr83hh85rg22ysxm9h-coreutils-8.32/bin/install: cannot create regular file '/etc/rpc': Permission denied
make[2]: *** [Makefile:197: /etc/rpc] Error 1
```

guix: consolidate Linux GCC package

Refactor our Linux GCC to be a single 'package', and avoid the use of
`package-with-extra-configure-variable`.

ci: Limit scope of some env vars

No need to have a larger scope than needed.

Can be reviewed with --color-moved=dimmed-zebra

ci: Start with clean env

This should help to avoid non-determinism.

ci: Remove no longer applicable section

This fails with:

"Error: determining starting point for build: no FROM statement found"

test: Fix intermittent issue in mempool_reorg

ci: Use concurrency for pull requests only

Otherwise, any previously pending workflow will be canceled on the
following push.

test: wallet_backup.py, fix intermittent failure in "restore using dumped wallet"

The failure arises because the test expects 'init_wallet()' (the test
framework function) creating a wallet with no keys. However, the function
also imports the deterministic private key used to receive the coinbase coins.

This causes a race within the "restore using dumped wallet" case, where we
intend to have a new wallet (with no existing keys) to test the
'importwallet()' RPC result.
The reason behind the intermittent failures might be other peers delivering
the chain right after node2 startup (sync of the validation queue included)
and prior to the 'node2.getbalance()' check.

test: previous releases: speed up fetching sources with shallow clone

For the sake of building previous releases, fetching the whole history
of the repository for each version seems to be overkill as it takes much
more time, bandwidth and disk space than necessary. Create a shallow
clone instead with history truncated to the one commit of the version
tag, which is directly checked out in the same command. This has the
nice side-effect that we can remove the extra `git checkout` step after
as it's not needed anymore.

Note that it might look confusing to pass a _tag_ to a parameter named
`--branch`, but the git-clone manpage explicitly states that this is
supported.

ci: Add missing docker.io prefix to CI_IMAGE_NAME_TAG

rpc: Add MaybeArg() and Arg() default helper

refactor: merge transport serializer and deserializer into Transport class

This allows state that is shared between both directions to be encapsulated
into a single object. Specifically the v2 transport protocol introduced by
BIP324 has sending state (the encryption keys) that depends on received
messages (the DH key exchange). Having a single object for both means it can
hide logic from callers related to that key exchange and other interactions.

net: add V1Transport lock protecting receive state

Rather than relying on the caller to prevent concurrent calls to the
various receive-side functions of Transport, introduce a private m_cs_recv
inside the implementation to protect the lock state.

Of course, this does not remove the need for callers to synchronize calls
entirely, as it is a stateful object, and e.g. the order in which Receive(),
Complete(), and GetMessage() are called matters. It seems impossible to use
a Transport object in a meaningful way in a multi-threaded way without some
form of external synchronization, but it still feels safer to make the
transport object itself responsible for protecting its internal state.

refactor: rename Transport class receive functions

Now that the Transport class deals with both the sending and receiving side
of things, make the receive side have function names that clearly indicate
they're about receiving.

* Transport::Read() -> Transport::ReceivedBytes()
* Transport::Complete() -> Transport::ReceivedMessageComplete()
* Transport::GetMessage() -> Transport::GetReceivedMessage()
* Transport::SetVersion() -> Transport::SetReceiveVersion()

Further, also update the comments on these functions to (among others) remove
the "deserialization" terminology. That term is better reserved for just the
serialization/deserialization between objects and bytes (see serialize.h), and
not the conversion from/to wire bytes as performed by the Transport.

net: abstract sending side of transport serialization further

This makes the sending side of P2P transports mirror the receiver side: caller provides
message (consisting of type and payload) to be sent, and then asks what bytes must be
sent. Once the message has been fully sent, a new message can be provided.

This removes the assumption that P2P serialization of messages follows a strict structure
of header (a function of type and payload), followed by (unmodified) payload, and instead
lets transports decide the structure themselves.

It also removes the assumption that a message must always be sent at once, or that no
bytes are even sent on the wire when there is no message. This opens the door for
supporting traffic shaping mechanisms in the future.

net: make V1Transport implicitly use current chainparams

The rest of net.cpp already uses Params() to determine chainparams in many
places (and even V1Transport itself does so in some places).

Since the only chainparams dependency is through the message start characters,
just store those directly in the transport.

fuzz: add bidirectional fragmented transport test

This adds a simulation test, with two V1Transport objects, which send messages
to each other, with sending and receiving fragmented into multiple pieces that
may be interleaved. It primarily verifies that the sending and receiving side
are compatible with each other, plus a few sanity checks.

net: measure send buffer fullness based on memory usage

This more accurately captures the intent of limiting send buffer size, as
many small messages can have a larger overhead that is not counted with the
current approach.

It also means removing the dependency on the header size (which will become
a function of the transport choice) from the send buffer calculations.

net: move message conversion to wire bytes from PushMessage to SocketSendData

This furthers transport abstraction by removing the assumption that a message
can always immediately be converted to wire bytes. This assumption does not hold
for the v2 transport proposed by BIP324, as no messages can be sent before the
handshake completes.

This is done by only keeping (complete) CSerializedNetMsg objects in vSendMsg,
rather than the resulting bytes (for header and payload) that need to be sent.
In SocketSendData, these objects are handed to the transport as permitted by it,
and sending out the bytes the transport tells us to send. This also removes the
nSendOffset member variable in CNode, as keeping track of how much has been sent
is now a responsability of the transport.

This is not a pure refactor, and has the following effects even for the current
v1 transport:

* Checksum calculation now happens in SocketSendData rather than PushMessage.
  For non-optimistic-send messages, that means this computation now happens in
  the network thread rather than the message handler thread (generally a good
  thing, as the message handler thread is more of a computational bottleneck).
* Checksum calculation now happens while holding the cs_vSend lock. This is
  technically unnecessary for the v1 transport, as messages are encoded
  independent from one another, but is untenable for the v2 transport anyway.
* Statistics updates about per-message sent bytes now happen when those bytes
  are actually handed to the OS, rather than at PushMessage time.

refactor: make Transport::ReceivedBytes just return success/fail

fuzz: coinselection, add `CreateCoins`

Move coins creation for a specific function. It
allows us to use it in other parts of the code.

fuzz: coinselection, add coverage for `EligibleForSpending`

fuzz: coinselection, add coverage for `AddInputs`

fuzz: coinselection, add coverage for `GetShuffledInputVector`/`GetInputSet`

fuzz: coinselection, add coverage for `Merge`

fuzz: coinselection, improve `ComputeAndSetWaste`

Instead of using `cost_of_change` for `min_viable_change`
and `change_cost`, and 0 for `change_fee`, use values from
`coin_params`. The previous values don't generate any effects
that is relevant for that context.

fuzz: coinselection, compare `GetSelectedValue` with target

The valid results should have a target below the sum of
the selected inputs amounts. Also, it increases the
minimum value for target to make it more realistic.

fuzz: coinselection, BnB should never produce change

fuzz: coinselection, fix `m_cost_of_change`

`m_cost_of_change` must not be generated randomly
independent from m_change_fee. This commit changes
it to set it up according to `wallet/spend`.

doc: Improve documentation of rpcallowip rpchelp

Closes #21070

v21.0 introduced a behaviour changed noted in #21070 where using a config value
`rpcallowip=::0` no longer also permitted ipv4 ip addresses.

The rpc_bind.py functional test covers this new behaviour already by checking
that the list of bind addresses exactly matches what is expected so this
commit only updates the documentation.

rpc: add test-only sendmsgtopeer rpc

This rpc can be used when we want a node to send a message, but
cannot use a python P2P object, for example for testing of low-level
net transport behavior.

test: add basic tests for sendmsgtopeer to rpc_net.py

test: add functional test for deadlock situation

guix: backport glibc patch to fix powerpc build

Do this prior to bumping the time-machine, to avoid the following build
failure:
```bash
 /tmp/guix-build-glibc-cross-powerpc64le-linux-gnu-2.27.drv-0/build/string/memset-power8.o.dt -MT /tmp/guix-build-glibc-cross-powerpc64le-linux-gnu-2.27.drv-0/build/string/memset-power8.o
../sysdeps/powerpc/powerpc64/power4/memcmp.S: Assembler messages:
../sysdeps/powerpc/powerpc64/power4/memcmp.S:87: Error: unrecognized opcode: `ldbrx'
../sysdeps/powerpc/powerpc64/power4/memcmp.S:88: Error: unrecognized opcode: `ldbrx'
../sysdeps/powerpc/powerpc64/power4/memcmp.S:112: Error: unrecognized opcode: `ldbrx'
```

See:
https://sourceware.org/git/?p=glibc.git;a=commit;h=9250e6610fdb0f3a6f238d2813e319a41fb7a810.
https://github.com/gcc-mirror/gcc/commit/e154242724b084380e3221df7c08fcdbd8460674.

guix: update python-oscrypto to 1.3.0

This is required for bumping the time-machine, for compatibility with
OpenSSL:

oscrypto: openssl backend, 1.2.1, /tmp/guix-build-python-oscrypto-1.2.1.drv-0/source/oscrypto
Traceback (most recent call last):
  File "/tmp/guix-build-python-oscrypto-1.2.1.drv-0/source/oscrypto/_openssl/_libcrypto_ctypes.py", line 304, in <module>
    libcrypto.EVP_PKEY_size.argtypes = [
  File "/gnu/store/9dkl9fnidcdpw19ncw5pk0p7dljx7ijb-python-3.10.7/lib/python3.10/ctypes/__init__.py", line 387, in __getattr__
    func = self.__getitem__(name)
  File "/gnu/store/9dkl9fnidcdpw19ncw5pk0p7dljx7ijb-python-3.10.7/lib/python3.10/ctypes/__init__.py", line 392, in __getitem__
    func = self._FuncPtr((name_or_ordinal, self))
AttributeError: /gnu/store/2hr7w64zhr6jjznidyc2xi40d5ynhj9c-openssl-3.0.8/lib/libcrypto.so.3: undefined symbol: EVP_PKEY_size. Did you mean: 'EVP_PKEY_free'?

guix: update time-machine to 160f78a4d92205df986ed9efcce7d3aac188cb24

In our time-machine environment this changes the following:

GCC 10.3.0 -> 10.4.0
Binutils 2.37 -> 2.38
Linux Libre Headers 5.15.37 -> 5.15.127
git 2.36.0 -> 2.41.0
mingw-w64 8.0.0 -> 11.0.1
NSIS 3.05 -> 3.09
xorriso 1.5.2 -> 1.5.6.pl02
Python 3.9 -> 3.10.7
Python-asn1crypto 1.4.0 -> 1.5.1

GCC 12.3.0 becomes available.
LLVM 15.0.7 becomes available.

guix: use cross-* keyword arguments

Using the new time-machine results in warnings about consistently using
keyword arguments:
```bash
guix environment: warning: 'cross-kernel-headers' must be used with keyword arguments
guix environment: warning: 'cross-libc' must be used with keyword arguments
```

guix: drop NSIS patch now that we use 3.09

See https://sourceforge.net/p/nsis/bugs/1283/.

guix: drop Windows broken-longjmp.patch

This is no-longer required, now that we are building using GCC 10.4.0.

depends: use LLVM/Clang 15.0.6 for macOS cross-compile

There is no x86_64 binaries for 15.0.7.

guix: use clang-toolchain-15 for macOS compilation

ci: Run "Win64 native" job on GitHub Actions

refactor: Use HashWriter over legacy CHashWriter

refactor: Use HashWriter over legacy CHashWriter (via SerializeHash)

config: default acceptnonstdtxn=0 on all chains

Previously, the default for acceptnonstdtxn defaulted to 0 on all
chains except testnet. Change this to be consistent across all
chains, and remove the parameter from chainparams entirely.

doc: Release notes for testnet defaulting to -acceptnonstdtxn=0

script: replace deprecated pkg_resources with importlib.metadata

in our python linter:

```
./test/lint/lint-python.py:12: DeprecationWarning: pkg_resources is deprecated as an API.
  See https://setuptools.pypa.io/en/latest/pkg_resources.html
  import pkg_resources
```

The importlib.metadata library was added in Python 3.8, which is currently our
minimum-supported Python version.

For more details about importlib.metadata, see https://docs.python.org/3/library/importlib.metadata.html

ci, windows: Do not run extended functional tests for pull requests

This change is intended to speed up the CI feedback for pull requests.

test: Support powerpc64le in get_previous_releases.py

guix: remove GCC 10 workaround from NSIS

Fixed upstream in 3.06, see
https://github.com/kichik/nsis/commit/229b6136c41ba5caba25936f4927476d20aa283f.
https://sourceforge.net/p/nsis/bugs/1248/

doc: Remove sudo from command that is already run as root

ci: Remove /ro_base bind mount

Just set the bind mount to BASE_READ_ONLY_DIR, which allows to drop one
line of code and makes the code easier to understand.

ci: Remove unused TEST_RUNNER_ENV="LC_ALL=C" from s390x task

gui: make '-min' minimize wallet loading dialog

When '-min' is enabled, no loading dialog should
be presented on screen during startup.

doc: Fill in the required skills in the good_first_issue template

[log] include wtxid in tx {relay,validation,orphanage} logging

[log] add category TXPACKAGES for orphanage and package relay

[log] add more logs related to orphan handling

- Whenever a tx is erased. Allows somebody to see which transactions
  have been erased due to expiry/overflow, not just how many.
- Whenever a tx is added to a peer's workset.
- AcceptToMemoryPool when a tx is accepted, mirroring the one logged for
  a tx received from a peer. This allows someone to see all of the
  transactions that are accepted to mempool just by looking for ATMP logs.
- MEMPOOLREJ when a tx is rejected, mirroring the one logged for
  a tx received from a peer. This allows someone to see all of the
  transaction rejections by looking at MEMPOOLREJ logs.

[doc] move comment about AlreadyHaveTx DoS score to the right place

This comment isn't in the right place, as detection of a tx in
recent_rejects would cause the function to exit much earlier.
Move the comment to the right place and tweak the first sentence for
accuracy.

ci: Avoid oversubscription in functional tests on Windows

ci: Avoid saving the same Ccache cache

This occurred when a job was being rerun.

log: Print error message when coindb is in inconsistent state

qt: Translation updates from Transifex

The diff is generated by executing the `update-translations.py` script.

qt: Bump Transifex slug for 26.x

qt: Update translation source file

The diff is generated by executing `make -C src translate`.

removed StrFormatInternalBug quote delimitation

ci: Bump `actions/checkout` version

See: https://github.com/actions/checkout/releases/tag/v4.0.0

test: p2p: check that `getaddr` msgs are only responded once per connection

test: remove fixed timeouts from feature_config_args

They cannot be scaled by the timeout_factor option and can
therefore cause timeouts in slow environments.
They are also not necessary for the test, since they measure time
frome startup until a debug message is encountered, which
is not restricted to 1 minute by any internal logic within bitcoind.

ci: Asan with -ftrivial-auto-var-init=pattern

Squashed 'src/secp256k1/' changes from c545fdc374..199d27cea3

199d27cea3 Merge bitcoin-core/secp256k1#1415: release: Prepare for 0.4.0
16339804c9 release: Prepare for 0.4.0
d9a85065a9 changelog: Catch up in preparation of release
0b4640aedd Merge bitcoin-core/secp256k1#1413: ci: Add `release` job
8659a01714 ci: Add `release` job
f9b38894ba ci: Update `actions/checkout` version
727bec5bc2 Merge bitcoin-core/secp256k1#1414: ci/gha: Add ARM64 QEMU jobs for clang and clang-snapshot
2635068abf ci/gha: Let MSan continue checking after errors in all jobs
e78c7b68eb ci/Dockerfile: Reduce size of Docker image further
2f0d3bbffb ci/Dockerfile: Warn if `ulimit -n` is too high when running Docker
4b8a647ad3 ci/gha: Add ARM64 QEMU jobs for clang and clang-snapshot
6ebe7d2bb3 ci/Dockerfile: Always use versioned clang packages
65c79fe2d0 Merge bitcoin-core/secp256k1#1412: ci: Switch macOS from Ventura to Monterey and add Valgrind
c223d7e33d ci: Switch macOS from Ventura to Monterey and add Valgrind
ea26b71c3a Merge bitcoin-core/secp256k1#1411: ci: Make repetitive command the default one
cce0456304 ci: Make repetitive command the default one
317a4c48f0 ci: Move `git config ...` to `run-in-docker-action`
4d7fe60905 Merge bitcoin-core/secp256k1#1409: ci: Move remained task from Cirrus to GitHub Actions
676ed8f9cf ci: Move "C++ (public headers)" from Cirrus to GitHub Actions
61fc3a2dc8 ci: Move "C++ -fpermissive..." from Cirrus to GitHub Actions
d51fb0a533 ci: Move "MSan" from Cirrus to GitHub Actions
c22ac27529 ci: Move sanitizers task from Cirrus to GitHub Actions
26a989924b Merge bitcoin-core/secp256k1#1410: ci: Use concurrency for pull requests only
ee1be62d84 ci: Use concurrency for pull requests only
6ee14550c8 Merge bitcoin-core/secp256k1#1406: ci, gha: Move more non-x86_64 tasks from Cirrus CI to GitHub Actions
fc3dea29ea ci: Move "ppc64le: Linux..." from Cirrus to GitHub Actions
7782dc8276 ci: Move "ARM64: Linux..." from Cirrus to GitHub Actions
0a16de671c ci: Move "ARM32: Linux..." from Cirrus to GitHub Actions
ea33914e00 ci: Move "s390x (big-endian): Linux..." from Cirrus to GitHub Actions
880be8af99 ci: Move "i686: Linux (Debian stable)" from Cirrus to GiHub Actions
2e6cf9bae5 Merge bitcoin-core/secp256k1#1396: ci, gha: Add "x86_64: Linux (Debian stable)" GitHub Actions job
5373693e45 Merge bitcoin-core/secp256k1#1405: ci: Drop no longer needed workaround
ef9fe959de ci: Drop no longer needed workaround
e10878f58e ci, gha: Drop `driver-opts.network` input for `setup-buildx-action`
4ad4914bd1 ci, gha: Add `retry_builder` Docker image builder
6617a620d9 ci: Remove "x86_64: Linux (Debian stable)" task from Cirrus CI
03c9e6508c ci, gha: Add "x86_64: Linux (Debian stable)" GitHub Actions job
ad3e65d9fe ci: Remove GCC build files and sage to reduce size of Docker image
6b9507adf6 Merge bitcoin-core/secp256k1#1398: ci, gha: Add Windows jobs based on Linux image
87d35f30c0 ci: Rename `cirrus.sh` to more general `ci.sh`
d6281dd008 ci: Remove Windows tasks from Cirrus CI
2b6f9cd546 ci, gha: Add Windows jobs based on Linux image
48b1d939b5 Merge bitcoin-core/secp256k1#1403: ci, gha: Ensure only a single workflow processes `github.ref` at a time
0ba2b94551 Merge bitcoin-core/secp256k1#1373: Add invariant checking for scalars
060e32cb60 Merge bitcoin-core/secp256k1#1401: ci, gha: Run all MSVC tests on Windows natively
de657c2044 Merge bitcoin-core/secp256k1#1062: Removes `_fe_equal_var`, and unwanted `_fe_normalize_weak` calls (in tests)
bcffeb14bc Merge bitcoin-core/secp256k1#1404: ci: Remove "arm64: macOS Ventura" task from Cirrus CI
c2f6435802 ci: Add comment about switching macOS to M1 on GHA later
4a24fae0bc ci: Remove "arm64: macOS Ventura" task from Cirrus CI
b0886fd35c ci, gha: Ensure only a single workflow processes `github.ref` at a time
3d05c86d63 Merge bitcoin-core/secp256k1#1394: ci, gha: Run "x86_64: macOS Ventura" job on GitHub Actions
d78bec7001 ci: Remove Windows MSVC tasks from Cirrus CI
3545dc2b9b ci, gha: Run all MSVC tests on Windows natively
5d8fa825e2 Merge bitcoin-core/secp256k1#1274: test: Silent noisy clang warnings about Valgrind code on macOS x86_64
8e54a346d2 ci, gha: Run "x86_64: macOS Ventura" job on GitHub Actions
b327abfcea Merge bitcoin-core/secp256k1#1402: ci: Use Homebrew's gcc in native macOS task
d62db57427 ci: Use Homebrew's gcc in native macOS task
54058d16fe field: remove `secp256k1_fe_equal_var`
bb4efd6404 tests: remove unwanted `secp256k1_fe_normalize_weak` call
eedd781085 Merge bitcoin-core/secp256k1#1348: tighten group magnitude limits, save normalize_weak calls in group add methods (revival of #1032)
b2f6712dd3 Merge bitcoin-core/secp256k1#1400: ctimetests: Use new SECP256K1_CHECKMEM macros also for ellswift
9c91ea41b1 ci: Enable ellswift module where it's missing
db32a24761 ctimetests: Use new SECP256K1_CHECKMEM macros also for ellswift
ce765a5b8e Merge bitcoin-core/secp256k1#1399: ci, gha: Run "SageMath prover" job on GitHub Actions
8408dfdc4c Revert "ci: Run sage prover on CI"
c8d9914fb1 ci, gha: Run "SageMath prover" job on GitHub Actions
8d2960c8e2 Merge bitcoin-core/secp256k1#1397: ci: Remove "Windows (VS 2022)" task from Cirrus CI
f1774e5ec4 ci, gha: Make MSVC job presentation more explicit
5ee039bb58 ci: Remove "Windows (VS 2022)" task from Cirrus CI
96294c00fb Merge bitcoin-core/secp256k1#1389: ci: Run "Windows (VS 2022)" job on GitHub Actions
a2f7ccdecc ci: Run "Windows (VS 2022)" job on GitHub Actions
374e2b54e2 Merge bitcoin-core/secp256k1#1290: cmake: Set `ENVIRONMENT` property for examples on Windows
1b13415df9 Merge bitcoin-core/secp256k1#1391: refactor: take use of `secp256k1_scalar_{zero,one}` constants (part 2)
a1bd4971d6 refactor: take use of `secp256k1_scalar_{zero,one}` constants (part 2)
b7c685e74a Save _normalize_weak calls in group add methods
c83afa66e0 Tighten group magnitude limits
26392da2fb Merge bitcoin-core/secp256k1#1386: ci: print $ELLSWIFT in cirrus.sh
d23da6d557 use secp256k1_scalar_verify checks
4692478853 ci: print $ELLSWIFT in cirrus.sh
c7d0454932 add verification for scalars
c734c64278 Merge bitcoin-core/secp256k1#1384: build: enable ellswift module via SECP_CONFIG_DEFINES
ad152151b0 update max scalar in scalar_cmov_test and fix schnorrsig_verify exhaustive test
78ca880788 build: enable ellswift module via SECP_CONFIG_DEFINES
0e00fc7d10 Merge bitcoin-core/secp256k1#1383: util: remove unused checked_realloc
b097a466c1 util: remove unused checked_realloc
2bd5f3e618 Merge bitcoin-core/secp256k1#1382: refactor: Drop unused cast
4f8c5bd761 refactor: Drop unused cast
173e8d061a Implement current magnitude assumptions
49afd2f5d8 Take use of _fe_verify_magnitude in field_impl.h
4e9661fc42 Add _fe_verify_magnitude (no-op unless VERIFY is enabled)
690b0fc05a add missing group element invariant checks
175db31149 ci: Drop no longer needed `PATH` variable update on Windows
116d2ab3df cmake: Set `ENVIRONMENT` property for examples on Windows
cef373997c cmake, refactor: Use helper function instead of interface library
747ada3587 test: Silent noisy clang warnings about Valgrind code on macOS x86_64

git-subtree-dir: src/secp256k1
git-subtree-split: 199d27cea32203b224b208627533c2e813cd3b21

scripted-diff: Use blocks_path where possible

-BEGIN VERIFY SCRIPT-
  sed -i 's|].chain_path, .blocks.|].blocks_path|g' $(git grep -l chain_path)
-END VERIFY SCRIPT-

index: Drop legacy -txindex check

move-only: Move CBlockTreeDB to node/blockstorage

The block index (CBlockTreeDB) is required to write and read blocks, so
move it to blockstorage. This allows to drop the txdb.h include from
`node/blockstorage.h`.

Can be reviewed with:

--color-moved=dimmed-zebra  --color-moved-ws=ignore-all-space

Fixup style of moved code

Can be reviewed with --word-diff-regex=.

scripted-diff: Rename CBlockTreeDB -> BlockTreeDB

-BEGIN VERIFY SCRIPT-
 sed -i 's|CBlockTreeDB|BlockTreeDB|g' $( git grep -l CBlockTreeDB )
-END VERIFY SCRIPT-

build: use -muse-unaligned-vector-move for Windows

We currently work around a longstanding GCC issue with aligned vector
instructions, in our release builds, by patching the behaviour we want
into GCC (see discussion in #24736).

A new option now exists in the binutils assembler,
`-muse-unaligned-vector-move`, which should also achieve the behaviour
we want (at least for our code). This was added in the 2.38 release,
see
https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=c8480b58e1968f209b6365af7422678f348222c2.
```bash
x86: Add -muse-unaligned-vector-move to assembler

Unaligned load/store instructions on aligned memory or register are as
fast as aligned load/store instructions on modern Intel processors.  Add
a command-line option, -muse-unaligned-vector-move, to x86 assembler to
encode encode aligned vector load/store instructions as unaligned
vector load/store instructions.
```

Even if we introduce this option into our build system, we'll have to
maintain our GCC patching, as we want all code that ends up in the
binary, to avoid these instructions. However, there may be some value in
adding the option, as it could be an improvement for someone building
(bitcoind.exe) with an unpatched compiler.

test: Combine sync_send_with_ping and sync_with_ping

miniscript: make GetStackSize independent of P2WSH context

It was taking into account the P2WSH script push in the number of stack
elements.

miniscript: introduce a helper to get the maximum witness size

Similarly to how we compute the maximum stack size.

Also note how it would be quite expensive to recompute it recursively
by accounting for different ECDSA signature sizes. So we just assume
high-R everywhere. It's only a trivial difference anyways.

descriptor: introduce a method to get the satisfaction size

In the wallet code, we are currently estimating the size of a signed
input by doing a dry run of the signing logic. This is unnecessary as
all outputs we are able to sign for can be represented by a descriptor,
and we can derive the size of a satisfaction ("signature") from the
descriptor itself directly.
In addition, this approach does not scale: getting the size of a
satisfaction through a dry run of the signing logic is only possible for
the most basic scripts.

This commit introduces the computation of the size of satisfaction per
descriptor. It's a bit intricate for 2 main reasons:
- We want to conserve the behaviour of the current dry-run logic used by
  the wallet that sometimes assumes ECDSA signatures will be low-r,
  sometimes not (when we don't create them).
- We need to account for the witness discount. A single descriptor may
  sometimes benefit of it, sometimes not (for instance `pk()` if used as
  top-level versus if used inside `wsh()`).

script/signingprovider: introduce a MultiSigningProvider

It is sometimes useful to interface with multiple signing providers at
once. For instance when inferring a descriptor with solving information
being provided from multiple sources (see next commit).

Instead of inneficiently copying the information from one provider into
the other, introduce a new signing provider that takes a list of
pointers to existing providers.

wallet: use descriptor satisfaction size to estimate inputs size

Instead of using the dummysigner to compute a placeholder satisfaction,
infer a descriptor on the scriptPubKey of the coin being spent and use
the estimation of the satisfaction size given by the descriptor
directly.

Note this (almost, see next paragraph) exactly conserves the previous
behaviour. For instance CalculateMaximumSignedInputSize was previously
assuming the input to be spent in a transaction that spends at least one
Segwit coin, since it was always accounting for the serialization of the
number of witness elements.

In this commit we use a placeholder for the size of the serialization of
the witness stack size (1 byte). Since the logic in this commit is
already tricky enough to review, and that it is only a very tiny
approximation not observable through the existing tests, it is addressed
in the next commit.

wallet: accurately account for the size of the witness stack

When estimating the maximum size of an input, we were assuming the
number of elements on the witness stack could be encode in a single
byte. This is a valid approximation for all the descriptors we support
(including P2WSH Miniscript ones), but may not hold anymore once we
support Miniscript within Taproot descriptors (since the max standard
witness stack size of 100 gets lifted).

It's a low-hanging fruit to account for it correctly, so just do it now.

fuzz: introduce and use `ConsumePrivateKey` helper

doc: s/--no-substitute/--no-substitutes in guix/INSTALL

Replace READWRITEAS macro with AsBase wrapping function

Co-authored-by: Pieter Wuille <pieter@wuille.net>

Rename CSerAction* to Action*

This allows new code, added in the next commit, to conform to the coding
guideline: No C-prefix for class names.

Support for serialization parameters

The moved part can be reviewed with the git options
 --ignore-all-space --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space

(Modified by Marco Falke)

Co-authored-by: Pieter Wuille <pieter@wuille.net>

Use serialization parameters for CAddress serialization

This also cleans up the addrman (de)serialization code paths to only
allow `Disk` serialization. Some unit tests previously forced a
`Network` serialization, which does not make sense, because Bitcoin Core
in production will always `Disk` serialize.
This cleanup idea was suggested by Pieter Wuille and implemented by Anthony
Towns.

Co-authored-by: Pieter Wuille <pieter@wuille.net>
Co-authored-by: Anthony Towns <aj@erisian.com.au>

test: add tests that exercise WithParams()

Co-authored-by: Vasil Dimov <vd@FreeBSD.org>

Remove unused legacy CHashVerifier

depends: libtapi 1300.0.6.5

depends: cctools 986 & ld64 711

test: remove unused variables in `p2p_invalid_block`

fuzz: add ConstructPubKeyBytes function

Today, this code only has one spot where it needs well-formed pubkeys,
but future PRs will want to reuse this code.

Add a function which creates a well-formed byte array that can be turned
into a pubkey. It is not required that the pubkey is valid, just that it
can be recognized as a compressed or uncompressed pubkey.

Note: while the main intent of this commit is to wrap the existing
logic into a function, it also switches to `PickValueFromArray` so that
we are only choosing one of 0x04, 0x06, or 0x07. The previous code,
`ConsumeIntegralInRange` would have also picked 0x05, which is not
definied in the context of compressed vs uncompressed keys.

See https://bitcoin.stackexchange.com/questions/57855/c-secp256k1-what-do-prefixes-0x06-and-0x07-in-an-uncompressed-public-key-signif
for more details.

net: add have_next_message argument to Transport::GetBytesToSend()

Before this commit, there are only two possibly outcomes for the "more" prediction
in Transport::GetBytesToSend():
* true: the transport itself has more to send, so the answer is certainly yes.
* false: the transport has nothing further to send, but if vSendMsg has more message(s)
         left, that still will result in more wire bytes after the next
         SetMessageToSend().

For the BIP324 v2 transport, there will arguably be a third state:
* definitely not: the transport has nothing further to send, but even if vSendMsg has
                  more messages left, they can't be sent (right now). This happens
                  before the handshake is complete.

To implement this, we move the entire decision logic to the Transport, by adding a
boolean to GetBytesToSend(), called have_next_message, which informs the transport
whether more messages are available. The return values are still true and false, but
they mean "definitely yes" and "definitely no", rather than "yes" and "maybe".

net: remove unused Transport::SetReceiveVersion

crypto: Spanify EllSwiftPubKey constructor

net: add V2Transport class with subset of BIP324 functionality

This introduces a V2Transport with a basic subset of BIP324 functionality:
* no ability to send garbage (but receiving is supported)
* no ability to send decoy packets (but receiving them is supported)
* no support for short message id encoding (neither encoding or decoding)
* no waiting until 12 non-V1 bytes have been received
* (and thus) no detection of V1 connections on the responder side
  (on the sender side, detecting V1 is not supported either, but that needs
  to be dealt with at a higher layer, by reconnecting)

net: make V2Transport auto-detect incoming V1 and fall back to it

net: add short message encoding/decoding support to V2Transport

net: make V2Transport send uniformly random number garbage bytes

net: make V2Transport preallocate receive buffer space

test: add unit tests for V2Transport

net: detect wrong-network V1 talking to V2Transport

Remove version/hashing options from CBlockLocator/CDiskBlockIndex

refactor: Use DataStream now that version/type are unused

refactor: remove clientversion include from dbwrapper.h

consensus/validation.h: remove needless GetTransactionOutputWeight helper

Introduced in 9b7ec393b82ca9d7ada77d06e0835df0386a8b85. This copied the format of the other Get.*Weight helpers but it's useless for a CTxOut.

test: refactor: remove unnecessary blocks_checked counter

Since we already store all the blocks in `events`, keeping an
additional counter is redundant.

test: refactor:  rename inbound to is_inbound

Makes it easier to recognize this variable represents a flag.

test: refactor: deduplicate handle_utxocache_* logic

Carve out the comparison logic into a helper function to avoid
code duplication.

test: store utxocache events

By storing the events instead of doing the comparison inside the
handle_utxocache_* functions, we simplify the overall logic and
potentially making debugging easier, by allowing pdb to access the
events.

Mostly a refactor, but changes logging behaviour slightly by not
raising and not calling self.log.exception("Assertion failed")

test: log sanity check assertion failures

test: refactor: remove unnecessary nonlocal

Since we're only mutating, and not reassigning, we don't need to
declare `events` as `nonlocal`.

test: refactor: usdt_mempool: store all events

Even though we expect these functions to only produce one event,
we still keep a counter to check if that's true. By simply storing
all the events, we can remove the counters and make debugging
easier, by allowing pdb to access the events.

net: merge V2Transport constructors, move key gen

This removes the ability for BIP324Cipher to generate its own key, moving that
responsibility to the caller (mostly, V2Transport). This allows us to write
the random-key V2Transport constructor by delegating to the explicit-key one.

net: do not use send buffer to store/cache garbage

Before this commit the V2Transport::m_send_buffer is used to store the
garbage:
* During MAYBE_V1 state, it's there despite not being sent.
* During AWAITING_KEY state, while it is being sent.
* At the end of the AWAITING_KEY state it cannot be wiped as it's still
  needed to compute the garbage authentication packet.

Change this by introducing a separate m_send_garbage field, taking over
the first and last role listed above. This means the garbage is only in
the send buffer when it's actually being sent, removing a few special
cases related to this.

doc: fix typos and mistakes in BIP324 code comments

index: coinstats reorg, fail when block cannot be reversed

During a reorg, continuing execution when a block cannot be
reversed leaves the coinstats index in an inconsistent state,
which was surely overlooked when 'CustomRewind' was implemented.

index: add [nodiscard] attribute to functions writing to the db

rpc: Deprecate rpcserialversion=0

doc: Clarify that -fstack-reuse=all bugs exist on all versions of GCC

wallet rpc: return final tx hex from walletprocesspsbt if complete

test: remove unnecessary finalizepsbt rpc calls

doc: add release note for PR #28414

doc, refactor: Changing -torcontrol help to specify that a default port is used

Right now when we get the help for -torcontrol it says that there is a
default ip and port we dont specify if there is a specified ip that we
would also use port 9051 as default

[policy] check for duplicate txids in package

Duplicates of normal transactions would be found by looking for
conflicting inputs, but this doesn't catch identical empty transactions.
These wouldn't be valid but exiting early is good and AcceptPackage's
result sanity checks assume non-duplicate transactions.

[CCoinsViewMemPool] track non-base coins and allow Reset

Temporary coins should not be available in separate subpackage submissions.
Any mempool coins that are cached in m_view should be removed whenever
mempool contents change, as they may be spent or no longer exist.

[validation] add AcceptSubPackage to delegate Accept* calls and clean up m_view

(1) Call AcceptSingleTransaction when there is only 1 transaction in the
  subpackage. This avoids calling PackageMempoolChecks() which enforces
rules that don't need to be applied for a single transaction, i.e.
disabling CPFP carve out.
There is a slight change in the error type returned, as shown in the
txpackage_tests change.  When a transaction is the last one left in the
package and its fee is too low, this returns a PCKG_TX instead of
PCKG_POLICY. This interface is clearer; "package-fee-too-low" for 1
transaction would be a bit misleading.

(2) Clean up m_view and m_viewmempool so that coins created in this
sub-package evaluation are not available for other sub-package
evaluations. The contents of the mempool may change, so coins that are
available now might not be later.

[validation] make PackageMempoolAcceptResult members mutable

After the PackageMempoolAcceptResult is returned from
AcceptMultipleTransactions, leave room for results to change due to
LimitMempool() eviction.

[refactor] back-fill results in AcceptPackage

Instead of populating the last PackageMempoolAcceptResult with stuff
from results_final and individual_results_nonfinal, fill results_final
and create a PackageMempoolAcceptResult using that one.

A future commit will add LimitMempoolSize() which may change the status
of each of these transactions from "already in mempool" or "submitted to
mempool" to "no longer in mempool". We will change those transactions'
results here.

A future commit also gets rid of the last AcceptSubPackage outside of
the loop. It makes more sense to use results_final as the place where
all results end up.

[validation] return correct result when already-in-mempool tx gets evicted

Bug fix: a transaction may be in the mempool when package evaluation
begins (so it is added to results_final with MEMPOOL_ENTRY or
DIFFERENT_WITNESS), but get evicted due to another transaction
submission.

[validation] don't LimitMempoolSize in any subpackage submissions

Don't do any mempool evictions until package validation is done,
preventing the mempool minimum feerate from changing. Whether we submit
transactions separately or as a package depends on whether they meet the
mempool minimum feerate threshold, so it's best that the value not
change while we are evaluating a package.
This avoids a situation where we have a CPFP package in which
the parents meet the mempool minimum feerate and are submitted by
themselves, but they are evicted before we have submitted the child.

[test framework] add ability to spend only confirmed utxos

Useful to ensure that the topologies of packages/transactions are as
expected, preventing bugs caused by having unexpected mempool ancestors.

[refactor] split setup in mempool_limit test

We want to be able to re-use fill_mempool so that none of the tests
affect each other.

Change the logs from info to debug because they are otherwise repeated
many times in the test output.

[test] mempool coins disappearing mid-package evaluation

Test for scenario(s) outlined in PR 28251.
Test what happens when a package transaction spends a mempool coin which
is fetched and then disappears mid-package evaluation due to eviction or
replacement.

dbwrapper: Use DataStream for batch operations

Remove unused GetType() from CBufferedFile and CAutoFile

GetType() is only called in tests, so it is unused and can be removed.

scripted-diff: Rename CBufferedFile to BufferedFile

While touching all constructors in the previous commit, the class name
can be adjusted to comply with the style guide.

-BEGIN VERIFY SCRIPT-
 sed -i 's/CBufferedFile/BufferedFile/g' $( git grep -l CBufferedFile )
-END VERIFY SCRIPT-

ci: Add test-each-commit task

ci: Limit test-each-commit to --max-count=6

[refactor] Allow std::array<std::byte, N> in serialize.h

This is already possible for C-style arrays, so allow it for C++11
std::array as well.

[refactor] Define MessageStartChars as std::array

kernel: Move MessageStartChars to its own file

The protocol.h file contains many non-consensus related definitions and
should thus not be part of the libbitcoinkernel. This commit makes
protocol.h no longer a required include for users of the
libbitcoinkernel.

This commit is part of the libbitcoinkernel project, namely its stage 1
step 3: Decouple most non-consensus headers from libbitcoinkernel.

Co-Authored-By: Cory Fields <cory-nospam-@coryfields.com>

[refactor] Add missing includes for next commit

[refactor] Add CChainParams member to CConnman

This is done in preparation to the next commit, but has the nice
effect of removing one further data structure relying on the global
`Params()`.

[refactor] Remove netaddress.h from kernel headers

Move functions requiring the netaddress.h include out of
libbitcoinkernel source files.

The netaddress.h file contains many non-consensus related definitions
and should thus not be part of the libbitcoinkernel. This commit makes
netaddress.h no longer a required include for users of the
libbitcoinkernel.

This commit is part of the libbitcoinkernel project, namely its stage 1
step 3: Decouple most non-consensus headers from libbitcoinkernel.

[refactor] Remove compat.h from kernel headers

This commit makes compat.h no longer a required include for users of the
libbitcoinkernel. Including compat.h imports a bunch of
platform-specific definitions.

This commit is part of the libbitcoinkernel project, namely its stage 1
step 3: Decouple most non-consensus headers from libbitcoinkernel.

ci: clang-17 for fuzz and tsan

[fuzz] Use afl++ shared-memory fuzzing

Using shared-memory is faster than reading from stdin, see
https://github.com/AFLplusplus/AFLplusplus/blob/7d2122e0596132f9344a5d0896020ebc79cd33db/instrumentation/README.persistent_mode.md

Revert "Merge bitcoin/bitcoin#28279: ci: Add test-each-commit task"

This reverts commit…
kwvg added a commit to kwvg/dash that referenced this pull request Feb 19, 2024
kwvg added a commit to kwvg/dash that referenced this pull request Feb 20, 2024
kwvg added a commit to kwvg/dash that referenced this pull request Feb 23, 2024
kwvg added a commit to kwvg/dash that referenced this pull request Feb 24, 2024
kwvg added a commit to kwvg/dash that referenced this pull request Feb 27, 2024
kwvg added a commit to kwvg/dash that referenced this pull request Feb 27, 2024
kwvg added a commit to kwvg/dash that referenced this pull request Feb 29, 2024
kwvg added a commit to kwvg/dash that referenced this pull request Mar 5, 2024
kwvg added a commit to kwvg/dash that referenced this pull request Mar 5, 2024
PastaPastaPasta added a commit to dashpay/dash that referenced this pull request Mar 6, 2024
, bitcoin#28267, bitcoin#28374, bitcoin#28100 (p2p primitives)

b60c493 merge bitcoin#28100: more Span<std::byte> modernization & follow-ups (Kittywhiskers Van Gogh)
c2aa01c merge bitcoin#28374: python cryptography required for BIP 324 functional tests (Kittywhiskers Van Gogh)
7c5edf7 merge bitcoin#28267: BIP324 ciphersuite follow-up (Kittywhiskers Van Gogh)
1b1924e merge bitcoin#28008: BIP324 ciphersuite (Kittywhiskers Van Gogh)
ff54219 merge bitcoin#27993: Make poly1305 support incremental computation + modernize (Kittywhiskers Van Gogh)
d7482eb merge bitcoin#27985: Add support for RFC8439 variant of ChaCha20 (Kittywhiskers Van Gogh)

Pull request description:

  ## Additional Information

  * Dependent on #5900

  * Dependent on #5901

  * Without modifications, tests introduced in [bitcoin#28008](bitcoin#28008) will fail due to salt comprising of a fixed string (`bitcoin_v2_shared_secret`) and network bytes ([source](https://github.com/sipa/bitcoin/blob/1c7582ead6e1119899922041c1af2b4169b0bc74/src/bip324.cpp#L39-L40)). Bitcoin uses [`{0xf9, 0xbe, 0xb4, 0xd9}`](https://github.com/sipa/bitcoin/blob/1c7582ead6e1119899922041c1af2b4169b0bc74/src/kernel/chainparams.cpp#L114-L117) for mainnet while Dash uses [`{0xbf, 0x0c, 0x6b, 0xbd}`](https://github.com/dashpay/dash/blob/37f43e4e56de0d510110a7f790df34ea77748dc9/src/chainparams.cpp#L238-L241).
    * The replacement parameters are generated by:
      * Cloning https://github.com/bitcoin/bips (as of this writing, at bitcoin/bips@b3701fa)
      * Editing `bip-0324/reference.py`
        * Changing `NETWORK_MAGIC` to Dash's network magic
      * Running `gen_test_vectors.py` to get a new `packet_encoding_test_vectors.csv`
      * Using [this python script](https://github.com/bitcoin/bitcoin/blob/1c7582ead6e1119899922041c1af2b4169b0bc74/src/test/bip324_tests.cpp#L174-L196) mentioned in a comment in `src/test/bip324_tests.cpp`, generate the values that will be used to replace the ones in `bip324_tests.cpp` (it will print to `stdout` so it's recommended to pipe it to a file)
      * Paste the new values over the old ones

  ## Breaking Changes

  None. Changes are restricted to BIP324 cryptosuite, tests and associated logic.

  ## Checklist:
    _Go over all the following points, and put an `x` in all the boxes that apply._
  - [x] I have performed a self-review of my own code
  - [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)**
  - [x] I have added or updated relevant unit/integration/functional/e2e tests **(note: N/A)**
  - [x] I have made corresponding changes to the documentation **(note: N/A)**
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

Top commit has no ACKs.

Tree-SHA512: bb056de8588026adae63e78d56878274ff3934a439e2eae81606fa9c0a37dab432a129315bb9c1b754400b5c883bf460eea3a0c857a3be0816c8fbf55c479843
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.

None yet

8 participants