| Age | Commit message (Collapse) | Author |
|---|
|
-BEGIN VERIFY SCRIPT-
perl -0777 -pi -e 's/#if defined\(HAVE_CONFIG_H\)\n#include <config\/bitcoin-config.h>.*\n#endif.*\n/#include <config\/bitcoin-config.h> \/\/ IWYU pragma: keep\n/g' $( git grep -l '#include <config/bitcoin-config.h>' )
-END VERIFY SCRIPT-
|
|
c6be144c4b774a03a8bcab5a165768cf81e9b06b Remove timedata (stickies-v)
92e72b5d0d49aa395e626c238bc28aba8e4c3d44 [net processing] Move IgnoresIncomingTxs to PeerManagerInfo (dergoegge)
7d9c3ec622d73a98d07ab3cee78751718982a5bc [net processing] Introduce PeerManagerInfo (dergoegge)
ee178dfcc1175e0af8163216c9c024f4bfc97965 Add TimeOffsets helper class (stickies-v)
55361a15d1aa6984051441bce88112000688fb43 [net processing] Use std::chrono for type-safe time offsets (stickies-v)
038fd979effb54ee76ce1b7cf078e920c652326a [net processing] Move nTimeOffset to net_processing (dergoegge)
Pull request description:
[An earlier approach](https://github.com/bitcoin/bitcoin/commits/1d226ae1f984c5c808f5c24c431b959cdefa692e/) in #28956 involved simplifying and refactoring the network-adjusted time calculation logic, but this was eventually [left out](https://github.com/bitcoin/bitcoin/pull/28956#issuecomment-1904214370) of the PR to make it easier for reviewers to focus on consensus logic changes.
Since network-adjusted time is now only used for warning/informational purposes, cleaning up the logic (building on @dergoegge's approach in #28956) should be quite straightforward and uncontroversial. The main changes are:
- Previously, we would only calculate the time offset from the first 199 outbound peers that we connected to. This limitation is now removed, and we have a proper rolling calculation. I've reduced the set to 50 outbound peers, which seems plenty.
- Previously, we would automatically use the network-adjusted time if the difference was < 70 mins, and warn the user if the difference was larger than that. Since there is no longer any automated time adjustment, I've changed the warning threshold to ~~20~~ 10 minutes (which is an arbitrary number).
- Previously, a warning would only be raised once, and then never again until node restart. This behaviour is now updated to 1) warn to log for every new outbound peer for as long as we appear out of sync, 2) have the RPC warning toggled on/off whenever we go in/out of sync, and 3) have the GUI warn whenever we are out of sync (again), but limited to 1 messagebox per 60 minutes
- no more globals
- remove the `-maxtimeadjustment` startup arg
Closes #4521
ACKs for top commit:
sr-gi:
Re-ACK [c6be144](https://github.com/bitcoin/bitcoin/pull/29623/commits/c6be144c4b774a03a8bcab5a165768cf81e9b06b)
achow101:
reACK c6be144c4b774a03a8bcab5a165768cf81e9b06b
dergoegge:
utACK c6be144c4b774a03a8bcab5a165768cf81e9b06b
Tree-SHA512: 1063d639542e882186cdcea67d225ad1f97847f44253621a8c4b36c4d777e8f5cb0efe86bc279f01e819d33056ae4364c3300cc7400c087fb16c3f39b3e16b96
|
|
packages
e518a8bf8abf3d7b83c9013f56d0dca18ae04d6f [functional test] opportunistic 1p1c package submission (glozow)
87c5c524d63c833cf490c7f2f73d72695ad480df [p2p] opportunistically accept 1-parent-1-child packages (glozow)
6c51e1d7d021ed6523107a6db87a865aaa8fc4c9 [p2p] add separate rejections cache for reconsiderable txns (glozow)
410ebd6efaf20fe4715c9b825103b74db69f35ac [fuzz] break out parent functions and add GetChildrenFrom* coverage (glozow)
d095316c1c23e9460dfbd9fdbaf292063adcd080 [unit test] TxOrphanage::GetChildrenFrom* (glozow)
2f51cd680fb4323f1c792dae37d4c4e0e0e35804 [txorphanage] add method to get all orphans spending a tx (glozow)
092c978a42e8f4a02291b994713505ba8aac8b28 [txpackages] add canonical way to get hash of package (glozow)
c3c1e15831c463df7968b028a77e787da7e6256d [doc] restore comment about why we check if ptx HasWitness before caching rejected txid (glozow)
6f4da19cc3b1b7cd23cb4be95a6bb9acb79eb3bf guard against MempoolAcceptResult::m_replaced_transactions (glozow)
Pull request description:
This enables 1p1c packages to propagate in the "happy case" (i.e. not reliable if there are adversaries) and contains a lot of package relay-related code. See https://github.com/bitcoin/bitcoin/issues/27463 for overall package relay tracking.
Rationale: This is "non-robust 1-parent-1-child package relay" which is immediately useful.
- Relaying 1-parent-1-child CPFP when mempool min feerate is high would be a subset of all package relay use cases, but a pretty significant improvement over what we have today, where such transactions don't propagate at all. [1]
- Today, a miner can run this with a normal/small maxmempool to get revenue from 1p1c CPFP'd transactions without losing out on the ones with parents below mempool minimum feerate.
- The majority of this code is useful for building more featureful/robust package relay e.g. see the code in #27742.
The first 2 commits are followups from #29619:
- https://github.com/bitcoin/bitcoin/pull/29619#discussion_r1523094034
- https://github.com/bitcoin/bitcoin/pull/29619#discussion_r1519819257
Q: What makes this short of a more full package relay feature?
(1) it only supports packages in which 1 of the parents needs to be CPFP'd by the child. That includes 1-parent-1-child packages and situations in which the other parents already pay for themselves (and are thus in mempool already when the package is submitted). More general package relay is a future improvement that requires more engineering in mempool and validation - see #27463.
(2) We rely on having kept the child in orphanage, and don't make any attempt to protect it while we wait to receive the parent. If we are experiencing a lot of orphanage churn (e.g. an adversary is purposefully sending us a lot of transactions with missing inputs), we will fail to submit packages. This limitation has been around for 12+ years, see #27742 which adds a token bucket scheme for protecting package-related orphans at a limited rate per peer.
(3) Our orphan-handling logic is somewhat opportunistic; we don't make much effort to resolve an orphan beyond asking the child's sender for the parents. This means we may miss packages if the first sender fails to give us the parent (intentionally or unintentionally). To make this more robust, we need receiver-side logic to retry orphan resolution with multiple peers. This is also an existing problem which has a proposed solution in #28031.
[1]: see this writeup and its links https://github.com/bitcoin/bips/blob/02ec218c7857ef60914e9a3d383b68caf987f70b/bip-0331.mediawiki#propagate-high-feerate-transactions
ACKs for top commit:
sr-gi:
tACK e518a8bf8abf3d7b83c9013f56d0dca18ae04d6f
instagibbs:
reACK e518a8bf8abf3d7b83c9013f56d0dca18ae04d6f
theStack:
Code-review ACK e518a8bf8abf3d7b83c9013f56d0dca18ae04d6f :package:
dergoegge:
light Code review ACK e518a8bf8abf3d7b83c9013f56d0dca18ae04d6f
achow101:
ACK e518a8bf8abf3d7b83c9013f56d0dca18ae04d6f
Tree-SHA512: 632579fbe7160cb763bbec6d82ca0dab484d5dbbc7aea90c187c0b9833b8d7c1e5d13b8587379edd3a3b4a02a5a1809020369e9cd09a4ebaf729921f65c15943
|
|
6a8b2befeab25e4e92d8e947a23e78014695e06c refactor: Avoid copying util::Result values (Ryan Ofsky)
834f65e82405bbed336f98996bc8cef366bbed0f refactor: Drop util::Result operator= (Ryan Ofsky)
Pull request description:
This PR just contains the first two commits of #25665.
It disables copying of `util::Result` objects because unnecessary copies are inefficient and not possible after #25665, which makes `util::Result` object move-only.
It disables the assignment operator and replaces it with an `Update()` method, because #25665 adds more information to `util::Result` objects (warning and error messages and failure values) and having an assignment operator that overwrites data instead of merging it would make it easy to accidentally erase existing information while trying to assign new information.
ACKs for top commit:
stickies-v:
re-ACK 6a8b2befeab25e4e92d8e947a23e78014695e06c
achow101:
ACK 6a8b2befeab25e4e92d8e947a23e78014695e06c
furszy:
re-ACK https://github.com/bitcoin/bitcoin/commit/6a8b2befeab25e4e92d8e947a23e78014695e06c
Tree-SHA512: 3f21af9031d50d6c68cca69133de03080f69b1ddcf8b140bdeb762069f14645209b2586037236d15b6ebd8973af0fbefd7e83144aeb7b84078a4cb4df812f984
|
|
to the mempool
cc15c5bfd1eb3903de246c124702a7c66c687008 fuzz: don't allow adding duplicate transactions to the mempool (Suhas Daftuar)
Pull request description:
Filter duplicate transaction ids from being added to the mempool in the `partially_downloaded_block` fuzz target.
I think a prerequisite for calling `CTxMemPool::addUnchecked` should be that the underlying txid doesn't already exist in the mempool (otherwise `addUnchecked` would need a way to return failure, which we don't currently have).
ACKs for top commit:
glozow:
utACK cc15c5bfd1eb3903de246c124702a7c66c687008 makes sense to me
maflcko:
lgtm ACK cc15c5bfd1eb3903de246c124702a7c66c687008
brunoerg:
ACK cc15c5bfd1eb3903de246c124702a7c66c687008
dergoegge:
utACK cc15c5bfd1eb3903de246c124702a7c66c687008
Tree-SHA512: 85f84ce405aba584e6d00391515f0a86c5648ce8b2da69036e50a6c1f6833d050d09b1972cc5ffbe7c4edb3e5f7f965ef34bd839deeddac27a889cc8d2e53b8f
|
|
30a6c999351041d6a1e8712a9659be1296a1b46a rpc: access some args by name (stickies-v)
bbb31269bfa449e82d3b6a20c2c3481fb3dcc316 rpc: add named arg helper (stickies-v)
13525e0c248eab9b199583cde76430c6da2426e2 rpc: add arg helper unit test (stickies-v)
Pull request description:
Adds string overloads for the `RPCHelpMan::Arg` and `RPCHelpMan::MaybeArg` helpers to be able to access RPC arguments by name instead of index number. Especially in RPCs with a large number of parameters, this can be quite helpful.
Example usage:
```cpp
const auto action{self.Arg<std::string>("action")};
```
Most of the LoC is adding test coverage and documentation updates. No behaviour change.
An alternative approach to #27788 with significantly less overhaul.
ACKs for top commit:
fjahr:
Code review ACK 30a6c999351041d6a1e8712a9659be1296a1b46a
maflcko:
ACK 30a6c999351041d6a1e8712a9659be1296a1b46a 🥑
ryanofsky:
Code review ACK 30a6c999351041d6a1e8712a9659be1296a1b46a. Nice change! Implementation is surprisingly simple and additional unit test coverage is welcome, too.
Tree-SHA512: 4904f5f914fe1d421d32f60edb7c5a028c8ea0f140a2f207a106b4752d441164e073066a6bf2e17693f859fe847815a96609d3cf521e0ac4178d8cd09362ea3d
|
|
|
|
18fd522ca9a74cf8690a6c9b9b729e78c6ed41fd ci, msvc: Add "Run fuzz binaries" step (Hennadii Stepanov)
52933d7283736fe3ae15e7ac44c02ca3bd95fe6d fuzz: Pass `SystemRoot` environment variable to subprocess (Hennadii Stepanov)
23cb8207cdd6c674480840b76626039cdfe7cb13 ci, msvc: Add "Clone fuzz corpus" step (Hennadii Stepanov)
19dceddf4bcdb74e84cf27229039a239b873d41b build, msvc: Build `fuzz.exe` binary (Hennadii Stepanov)
4c078d7bd278fa8b4db6e1da7b9b747f49a8ac4c build, msvc: Enable preprocessor conformance mode (Hennadii Stepanov)
09f5a74198c328c80539c17d951a70558e6b361e fuzz: Re-implement `read_stdin` in portable way (Hennadii Stepanov)
Pull request description:
Closes https://github.com/bitcoin/bitcoin/issues/29760.
Suggested in https://github.com/bitcoin/bitcoin/pull/29758#issuecomment-2025593572.
ACKs for top commit:
maflcko:
lgtm ACK 18fd522ca9a74cf8690a6c9b9b729e78c6ed41fd 🔍
sipsorcery:
tACK 18fd522ca9a74cf8690a6c9b9b729e78c6ed41fd
sipa:
utACK 18fd522ca9a74cf8690a6c9b9b729e78c6ed41fd
Tree-SHA512: 672ed6926ee9091f68f13780e77b60fc1d48731f16e847d849374f8426ffe1dafd9bcab06a27af62e8052ba345bb57f20f40579d6be8540c12ef85c23a6eec8b
|
|
It's very hard to randomly construct a transaction that would be the
parent of an existing orphanage tx. For functions like
AddChildrenToWorkSet and GetChildren that take orphan parents, use a tx
that was previously constructed.
|
|
|
|
|
|
|
|
`util::Result` objects are aggregates that can hold multiple fields with
different information. Currently Result objects can only hold a success value
of an arbitrary type or a single bilingual_str error message. In followup PR
https://github.com/bitcoin/bitcoin/pull/25722, Result objects may be able to
hold both success and failure values of different types, plus error and warning
messages.
Having a Result::operator= assignment operator that completely erases all
existing Result information before assigning new information is potentially
dangerous in this case. For example, code that looks like it is assigning a
warning value could erase previously-assigned success or failure values.
Conversely, code that looks like it is just assigning a success or failure
value could erase previously assigned error and warning messages.
To prevent potential bugs like this, disable Result::operator= assignment
operator.
It is possible in the future we may want to re-enable operator= in limited
cases (such as when implicit conversions are not used) or add a Replace() or
Reset() method that mimicks default operator= behavior. Followup PR
https://github.com/bitcoin/bitcoin/pull/25722 also adds a Result::Update()
method providing another way to update an existing Result object.
Co-authored-by: stickies-v <stickies-v@protonmail.com>
|
|
992c714451676cee33d3dff49f36329423270c1c common: Don't terminate on null character in UrlDecode (Fabian Jahr)
099fa571511f113e0056d4bc27b3153a42f9dc65 scripted-diff: Modernize name of urlDecode function and param (Fabian Jahr)
8f39aaae417c33490e0e41fb97620eb23ced3d05 refactor: Remove hooking code for urlDecode (Fabian Jahr)
650d43ec15f7a3ae38126f65ef8fa0b1fd3ee936 refactor: Replace libevent use in urlDecode with our own code (Fabian Jahr)
46bc6c2aaa613eef526b21a06bf21e8edde31a88 test: Add unit tests for urlDecode (Fabian Jahr)
Pull request description:
Fixes #29654 (as a side-effect)
Removing dependencies is a general goal of the project and the xz backdoor has been an additional wake up call recently. Libevent shows many of the same symptoms, few maintainers and slow releases. While libevent can not be removed completely over night we should start removing it’s usage where it's possible, ideally with the end goal to removing it completely.
This is a pretty easy win in that direction. The [`evhttp_uridecode` function from libevent](https://github.com/libevent/libevent/blob/e0a4574ba2cbcdb64bb2b593e72be7f7f4010746/http.c#L3542) we were using in `urlDecode` could be easily emulated in fewer LOC. This also ports the [applicable test vectors over from libevent](https://github.com/libevent/libevent/blob/master/test/regress_http.c#L3430).
ACKs for top commit:
achow101:
ACK 992c714451676cee33d3dff49f36329423270c1c
theStack:
Code-review ACK 992c714451676cee33d3dff49f36329423270c1c
maflcko:
ACK 992c714451676cee33d3dff49f36329423270c1c 👈
stickies-v:
ACK 992c714451676cee33d3dff49f36329423270c1c
Tree-SHA512: 78f76ae7ab3b6710eab2aaac20f55eb0da7803e057eaa6220e865f328666a5399ef1a479702aaf630b2f974ad3aa15e2b6adac9c11bc8c3d4be21e8af1667fea
|
|
The previous behavior was the result of casting the result returned from the libevent function evhttp_uridecode to std:string but this was probably not intended.
|
|
-BEGIN VERIFY SCRIPT-
sed -i 's/urlDecode/UrlDecode/g' $(git grep -l 'urlDecode' ./src)
sed -i 's/urlEncoded/url_encoded/g' $(git grep -l 'urlEncoded' ./src)
-END VERIFY SCRIPT-
|
|
The point of this was to be able to build bitcoin-tx and bitcoin-wallet without libevent, see #18504.
Now that we use our own implementation of urlDecode this is not needed anymore.
Co-authored-by: stickies-v <stickies-v@protonmail.com>
|
|
Co-authored-by: Lőrinc <pap.lorinc@gmail.com>
|
|
compare based on chunks
b22901dfa9cc3af94bf13163a28300eb1ab25b22 Avoid explicitly computing diagram; compare based on chunks (Pieter Wuille)
Pull request description:
This merges the `BuildDiagramFromChunks` and `CompareFeeRateDiagram` introduced in #29242 into a single `CompareChunks` function, which operates on sorted chunk data rather than diagrams, instead computing the diagram on the fly.
This avoids the need for the construction of an intermediary diagram object, and removes the slightly arbitrary "all diagrams must start at (0, 0)" requirement.
Not a big deal, but I think the result is a bit cleaner and not really more complicated.
ACKs for top commit:
glozow:
reACK b22901d
instagibbs:
reACK https://github.com/bitcoin/bitcoin/pull/29757/commits/b22901dfa9cc3af94bf13163a28300eb1ab25b22
Tree-SHA512: ca37bdf61d9a9cb5435f4da73e97ead33bf65828ad9af49b87336b1ece70db8ced1c21f517fc6eb6d616311c91f3da75ecae6b9bd42547133e3a3c5320b7816d
|
|
header name conventions
08f756bd370c3e100b186c7e24bef6a033575b29 Replace locale-dependent `std::strerror` with `SysErrorString` (Hennadii Stepanov)
d8e4ba4d0568769782b8c19c82e955c4aee73477 refactor: Rename `subprocess.hpp` to follow our header name conventions (Hennadii Stepanov)
Pull request description:
This PR renames the header `*.hpp` --> `*.h` and adjusts the header guard name, which makes it available for processing by linters.
Fixed the following linter warning:
```
The locale dependent function strerror(...) appears to be used:
src/util/subprocess.h: std::runtime_error( err_msg + ": " + std::strerror(err_code) )
Unnecessary locale dependence can cause bugs that are very tricky to isolate and fix. Please avoid using locale-dependent functions if possible.
Advice not applicable in this specific case? Add an exception by updating the ignore list in /bitcoin/test/lint/lint-locale-dependence.py
^---- failure generated from lint-locale-dependence.py
```
ACKs for top commit:
TheCharlatan:
ACK 08f756bd370c3e100b186c7e24bef6a033575b29
Tree-SHA512: 57a2f01c20eb9552481e428a4969bd59e9ada9f784fe1a45cb62aa9c9152c8e950d336854f45af0e2e5dc7c7b2a1fb216c8f832e3d6ccfb457ad71b6e423231e
|
|
TapMiniscript
4d8d21320eba54571ff63931509cd515c3e20339 sign: don't assume we are parsing a sane Miniscript (Antoine Poinsot)
Pull request description:
The script provided for signature might be externally provided, for instance by way of 'finalizepsbt'. Therefore the script might be ill-crafted, so don't assume pubkeys are always 32 bytes.
Thanks to Niklas for finding this.
FIxes https://github.com/bitcoin/bitcoin/issues/29851.
ACKs for top commit:
achow101:
ACK 4d8d21320eba54571ff63931509cd515c3e20339
furszy:
ACK 4d8d21320eba54571ff63931509cd515c3e20339 with a small nuance that could be tackled in a follow-up by someone else (or never).
Tree-SHA512: 29b7948b56e6dc05eac1014d684f2129ab1d19cb1e5d304216c826b7057c0e1d84ceb18731b91124b680e17d90e38de9f9a5526e4f6ecc3ea816881a6599bb47
|
|
|
|
The script provided for signature might be externally provided, for
instance by way of 'finalizepsbt'. Therefore the script might be
ill-crafted, so don't assume pubkeys are always 32 bytes.
Thanks to Niklas for finding this.
|
|
|
|
diagram checks
016ed248ba0ae64e3f0c93bb47a2cd9b5e49cd85 fuzz: explicitly cap the vsize of RBFs for diagram checks (Greg Sanders)
Pull request description:
In master we are hitting a case where vsize transactions much larger than max standard size are causing an overflow in not-yet-exposed RBF diagram checking code: https://github.com/bitcoin/bitcoin/pull/29757#issuecomment-2049220195
`ConsumeTxMemPoolEntry` is creating entries with tens of thousands of sigops cost, causing the resulting RBFs to be "overly large".
To fix this I cause the fuzz test to stop adding transactions to the mempool when we reach a potential overflow of `int32_t`.
ACKs for top commit:
glozow:
ACK 016ed248ba0ae64e3f0c93bb47a2cd9b5e49cd85
marcofleon:
ACK 016ed248ba0ae64e3f0c93bb47a2cd9b5e49cd85. I ran libFuzzer on `package_rbf` on the current master branch until the overflow was encountered. Then I built the PR branch and ran the fuzzer using the crash input.
Tree-SHA512: b3ffc98d2c4598eb3010edd58b9370aab1441aafbb1044c83b2b90c17dfe9135b8de9dba475dd0108863c1ffedede443cd978e95231a41cf1f0715629197fa51
|
|
|
|
|
|
|
|
|
|
being set as client_maxfeerate failure
4ba1d0b55339c3ea90e2bcd64662a06f0f90dd46 fuzz: Add coverage for client_maxfeerate (Greg Sanders)
91d7d8f22a1c528db14fa743c66cd861ea00e84b AcceptMultipleTransactions: Fix workspace client_maxfeerate (Greg Sanders)
f3aa5bd5eb6d1088f98a4dc7daaab0e17a7d5529 fill_mempool: assertions and docsctring update (Greg Sanders)
a3da63e8febe475f2250f6432bca237d31fa9107 Move fill_mempool to util function (Greg Sanders)
73b68bd8b4f9447e30091c7f8c3dc91a086bd93b fill_mempool: remove subtest-specific comment (Greg Sanders)
Pull request description:
Bug causes an `Assume()` failure due to the expectation that the individual result should be invalid when done over `submitpackage` via rpc.
Bug introduced by https://github.com/bitcoin/bitcoin/pull/28950 , and I discovered it rebasing https://github.com/bitcoin/bitcoin/pull/28984 since it's easier to hit in that test scenario.
Tests in place were only checking `AcceptSingleTransaction`-level checks due to package evaluation only triggering when minfee is too high for the parent transaction.
Added test along with fix, moving the fill_mempool utility into a common area for re-use.
ACKs for top commit:
glozow:
reACK 4ba1d0b55339c3ea90e2bcd64662a06f0f90dd46
theStack:
ACK 4ba1d0b55339c3ea90e2bcd64662a06f0f90dd46
ismaelsadeeq:
re-ACK https://github.com/bitcoin/bitcoin/commit/4ba1d0b55339c3ea90e2bcd64662a06f0f90dd46 via [diff](https://github.com/bitcoin/bitcoin/compare/4fe7d150eb3c85a6597d8fc910fe1490358197ad..4ba1d0b55339c3ea90e2bcd64662a06f0f90dd46)
Tree-SHA512: 3729bdf7f25d04e232f173ccee04ddbb2afdaafa3d04292a01cecf58fb11b3b2bc133e8490277f1a67622b62d17929c242dc980f9bb647896beea4332ee35306
|
|
With the introduction and usage of TimeOffsets, there is no more need
for this file. Remove it.
|
|
This helper class is an alternative to CMedianFilter, but without a
lot of the special logic and exceptions that we needed while it was
still used for consensus.
|
|
d5a715536e497c160a2520f81334aab6c7490213 build: remove boost::process dependency for building external signer support (Sebastian Falbesoner)
70434b1c443d9251a880d0193af771f574c40617 external_signer: replace boost::process with cpp-subprocess (Sebastian Falbesoner)
cc8b9875b104c31f0a5b5e4195a8278ec55f35f7 Add `cpp-subprocess` header-only library (Hennadii Stepanov)
Pull request description:
Closes https://github.com/bitcoin/bitcoin/issues/24907.
This PR is based on **theStack**'s [work](https://github.com/bitcoin/bitcoin/issues/24907#issuecomment-1466087049).
The `subprocess.hpp` header has been sourced from the [upstream repo](https://github.com/arun11299/cpp-subprocess) with the only modification being the removal of convenience functions, which are not utilized in our codebase.
Windows-related changes will be addressed in subsequent follow-ups.
ACKs for top commit:
achow101:
reACK d5a715536e497c160a2520f81334aab6c7490213
Sjors:
re-tACK d5a715536e497c160a2520f81334aab6c7490213
theStack:
Light re-ACK d5a715536e497c160a2520f81334aab6c7490213
fanquake:
ACK d5a715536e497c160a2520f81334aab6c7490213 - with the expectation that this code is going to be maintained as our own. Next PRs should:
Tree-SHA512: d7fb6fecc3f5792496204190afb7d85b3e207b858fb1a75efe483c05260843b81b27d14b299323bb667c990e87a07197059afea3796cf218ed8b614086bd3611
|
|
`UCharCast` calls
56e1e5dd10cbe51d3abc3fbf532b6b41bf62a889 refactor, bench, fuzz: Drop unneeded `UCharCast` calls (Hennadii Stepanov)
Pull request description:
The `CKey::Set()` template function handles `std::byte` just fine: https://github.com/bitcoin/bitcoin/blob/b5d21182e5a66110ce2796c2c99da39c8ebf0d72/src/key.h#L105
Noticed in https://github.com/bitcoin/bitcoin/pull/29774#discussion_r1546288181.
ACKs for top commit:
maflcko:
lgtm ACK 56e1e5dd10cbe51d3abc3fbf532b6b41bf62a889
laanwj:
Seems fine, code review ACK 56e1e5dd10cbe51d3abc3fbf532b6b41bf62a889
hernanmarino:
ACK 56e1e5dd10cbe51d3abc3fbf532b6b41bf62a889
Tree-SHA512: 0f6b6e66692e70e083c7768aa4859c7db11aa034f555d19df0e5d33b18c0367ba1c886bcb6be3fdea78248a3cf8285576120812da55b995ef5e6c94a9dbd9f7c
|
|
|
|
encodings
a8203e94123b6ea6e4f4a6320e3ad20457f44a28 refactor: Simplify `extra_txn` to be a vec of CTransactionRef instead of a vec of pair<Wtxid, CTransactionRef> (AngusP)
c3c18433ae1d5b024d4cb92c762f5ca0ec7849c8 refactor: Use typesafe Wtxid in compact block encoding message, instead of ambiguous uint256. (AngusP)
Pull request description:
The first commit replaces `uint256` with typesafe `Wtxid` (or `Txid`) types introduced in #28107.
The second commit then simplifies the extra tx `vector` to just be of `CTransactionRef`s instead of a `std::pair<Wtxid, CTransactionRef>`, as it's easy to get a `Wtxid` from a transaction ref.
ACKs for top commit:
glozow:
ACK a8203e94123b6ea6e4f4a6320e3ad20457f44a28
dergoegge:
ACK a8203e94123b6ea6e4f4a6320e3ad20457f44a28
Tree-SHA512: b4ba1423a8059f9fc118782bd8a668213d229c822f22b01267de74d6ea97fe4f2aad46b5da7c0178ecc9905293e9a0eafba1d75330297c055e27fd53c8c8ebfd
|
|
Co-authored-by: stickies-v <stickies-v@protonmail.com>
Co-authored-by: Gloria Zhao <gloriajzhao@gmail.com>
|
|
vec of pair<Wtxid, CTransactionRef>
All `CTransactionRef` have `.GetWitnessHash()` that returns a cached `const Wtxid` (since fac1223a568fa1ad6dd602350598eed278d115e8),
so we don't need to pass transaction refs around with their IDs as they're easy to get from a ref.
|
|
The `CKey::Set()` template function handles `std::byte` just fine.
|
|
561a650e0f669159699224ddd4eb5b1c91cf9ac3 test: Fix debug recommendation in argsman_tests (Fabian Jahr)
Pull request description:
There are recommendations in the `argsman_tests` comments on how to re-run and debug a test failure to see if it reflects an expected or unexpected change. The command tries to run a test in `util_tests` but this is in `argsman_tests` so the command doesn't work with just copy+paste. I didn't investigate further but I suspect that these tests were moved between files.
ACKs for top commit:
fanquake:
ACK 561a650e0f669159699224ddd4eb5b1c91cf9ac3
Tree-SHA512: b3bb94ba1635c9455149b455f2b30ee37a8067a6242339531ab54d428177a288da29a4a10702652305eb34aa7638f51dad35fa6b0e7b74617e445327b8c4c053
|
|
fa9f36babaceba6ab2f88e64bc4bc2956f58871f build: Remove HAVE_GMTIME_R (MarcoFalke)
fa72dcbfa56177ca878375bae7c7bca6ca6a1f40 refactor: FormatISO8601* without gmtime* (MarcoFalke)
fa2c486afc8501f2678cc19c9e9518a23c4ebcbd Revert "time: add runtime sanity check" (MarcoFalke)
Pull request description:
Now that the `ChronoSanityCheck` has passed for everyone with C++17 and is guaranteed by C++20 to always pass, remove it.
Also, remove `gmtime_r` and `gmtime_s` and replace them with `year_month_day`+`hh_mm_ss` from C++20.
ACKs for top commit:
sipa:
utACK fa9f36babaceba6ab2f88e64bc4bc2956f58871f
fanquake:
ACK fa9f36babaceba6ab2f88e64bc4bc2956f58871f - more std lib & even less stuff to port.
Tree-SHA512: a9e7e805b757b7dade0bcc3f95273a7dc4f68622630d74838339789dd203ad7542d36b2e090a93b2bc5a7ecc383207dd7ec82c68147108bdac7ce44f088c8c9a
|
|
|
|
80f8b92f4f2311b9e9a25361c9dd973244e6f95c remove libbitcoinconsensus (fanquake)
Pull request description:
This was deprecated in `v27.0`, for removal in `v28.0`. See discussion in PR #29189.
ACKs for top commit:
theuni:
Concept ACK and light review ACK 80f8b92f4f2311b9e9a25361c9dd973244e6f95c. My only hesitation here is that (afaics?) there's now nothing keeping undesired features like threading or globals from working their way into the interpreter in future commits.
m3dwards:
Concept ACK https://github.com/bitcoin/bitcoin/pull/29648/commits/80f8b92f4f2311b9e9a25361c9dd973244e6f95c
TheCharlatan:
ACK 80f8b92f4f2311b9e9a25361c9dd973244e6f95c
hebasto:
ACK 80f8b92f4f2311b9e9a25361c9dd973244e6f95c, I have reviewed the code and it looks OK.
Tree-SHA512: 17a62118aeb088f2695c892bb32794dfea3061e3cb7d9e8e9f1c06c3ff6f63a7587fa532e37edbb91fbc5a19b12c9a0f8e05fa9e8864aa07f92665375d847e80
|
|
ambiguous uint256.
Wtxid/Txid types introduced in #28107
|
|
This primarily affects the `RunCommandParseJSON` utility function.
|
|
|
|
|
|
|
|
|
|
|
