Age | Commit message (Collapse) | Author |
|
f865cf8ded2b2fbc82a6fbc41226d991909a6880 Add and use BlockManager::GetAllBlockIndices (Carl Dong)
28ba0313eac37e4a900b7e97af7169ce999c4024 Add and use CBlockIndexHeightOnlyComparator (Carl Dong)
12eb05df63f930969115af6dc66e2e5d02f2a517 move-only: Move CBlockIndexWorkComparator to blockstorage (Carl Dong)
c600ee38168a460d3026eae0e289c976194aad14 Only load BlockMan in BlockMan member functions (Carl Dong)
42e56d9b188f97c077ed2269e24acc0be35ece17 style-only: No need for std::pair for vSortedByHeight (Carl Dong)
3bbb6fea051f4e19bd2448e401a6c4e9b4cc7a41 style-only: Various blockstorage.cpp cleanups (Carl Dong)
5be9ee3c54dcb396ff52fc8c8b7e4e6e39ec4a3b refactor: more const annotations for uses of CBlockIndex* (Anthony Towns)
Pull request description:
The only important commit is "Only load BlockMan in BlockMan member functions", everything else is all just small style changes.
Here's the commit message, reproduced:
```
This commit effectively splits the "load block index itself" logic from
"derive Chainstate variables from loaded block index" logic.
This means that BlockManager::LoadBlockIndex{,DB} will only load what's
relevant to the BlockManager.
```
ACKs for top commit:
ajtowns:
ACK f865cf8ded2b2fbc82a6fbc41226d991909a6880 ; code review only
MarcoFalke:
review ACK f865cf8ded2b2fbc82a6fbc41226d991909a6880 🗂
Tree-SHA512: 7b204d782834e06fd7329d022e2ae860181b4e8105c33bfb928539a4ec24161dc7438a9c4d4ee279dcad77de310c160b997bb8aa18923243d0fd55ccf4ad7c3a
|
|
Can be reviewed with:
--color-moved=dimmed-zebra --color-moved-ws=ignore-all-space
|
|
|
|
|
|
4523d28b6bd550bf9f9c724f9aa90c3d2d4ebe3b [test] compare filter and header with the result of the getblockfilter RPC (Niklas Gögge)
3a2464f21619c3065ec2f5d8f7431703c30c964a [rest] drop superfluous rpc serializations flags for block filters (Niklas Gögge)
064abd14a55e0fa1bff530237816a748d01e0ddb [rest] add a more verbose error message for invalid header counts (Niklas Gögge)
83b8f3a8961baa34a136ecfaf62c3ea0d133b6d6 [refactor] various style fix-ups (Niklas Gögge)
Pull request description:
This PR addresses unresolved review comments from [#17631](https://github.com/bitcoin/bitcoin/pull/17631).
This includes:
* various style fix-ups
* returning a more verbose error message for invalid header counts
* removing superfluous rpc serializations flags for block filters
* improving the test to include comparing the block filters returned from the rest with the ones returned from the `getblockfilter` RPC.
ACKs for top commit:
jnewbery:
ACK 4523d28b6bd550bf9f9c724f9aa90c3d2d4ebe3b
brunoerg:
tACK 4523d28b6bd550bf9f9c724f9aa90c3d2d4ebe3b
Tree-SHA512: 634e6b2ae3e1d5f31675a50cfe11a5e74bf5a51b9e7b512d9e18879bf2ed424fc0ac6ec633023f192e3ad12cf0c73b0b51de79dd7ec00844dba3e1493d823b8c
|
|
-BEGIN VERIFY SCRIPT-
./contrib/devtools/copyright_header.py update ./
-END VERIFY SCRIPT-
Commits of previous years:
* 2020: fa0074e2d82928016a43ca408717154a1c70a4db
* 2019: aaaaad6ac95b402fe18d019d67897ced6b316ee0
|
|
|
|
|
|
|
|
2b64fa3251ac5ff4b4d174f1f0be7226490dce87 Update REST docs with new accessors (Matt Corallo)
ef7c8228fd5cf45526518ae2bd5ebdd483e65525 Expose block filters over REST. (Matt Corallo)
Pull request description:
This adds a new rest endpoint:
/rest/blockfilter/filtertype/requesttype/blockhash (eg
/rest/blockfilter/basic/header/000000000019d6689c085ae165831e934ff763ae46a2a6c172b3f1b60a8ce26f.hex)
which exposes either the filter "header" or the filter data itself.
Most of the code is cribbed from the equivalent RPC.
You can test it at http://bitcoin-rest.bitcoin.ninja/rest//blockfilter/basic/header/000000005b7a58a939b2636f61fa4ddd62258c5fed57667a35d23f2334c4f86d.hex
ACKs for top commit:
dergoegge:
ACK 2b64fa3251ac5ff4b4d174f1f0be7226490dce87 - Adding blockfilters to the REST interface is analogous to serving other public data such as transactions or blocks.
Tree-SHA512: d487bc694266375c94d6fcf2e9d788a8a42a3b94e8d3290e46335a64cbcde55084ce5ea6119b79a4065888d94d7c3ae25a59a901fa46e3711f0eb296add12696
|
|
|
|
(#21245 modified)
5c34507ecbbdc29c086276d1c62835b461823507 core_write: Rename calculate_fee to have_undo for clarity (fyquah)
8edf6204a87057a451160d1e61e79d8be112e81f release-notes: Add release note about getblock verbosity level 3. (fyquah)
459104b2aae6eeaadfa5a7e47944f1a34780dacd rest: Add test for prevout fields in getblock (fyquah)
4330af6f72172848f5971a052a8f325ed50eb576 rpc: Add test for level 3 verbosity getblock rpc call. (fyquah)
51dbc167e98daab317baa80cf80bfda337672dab rpc: Add level 3 verbosity to getblock RPC call. (fyquah)
3cc95345ca49b87e8caca9a0e6418c63ae1e463a rpc: Replace boolean argument for tx details with enum class. (fyquah)
Pull request description:
Author of #21245 expressed [time issues](https://github.com/bitcoin/bitcoin/pull/21245#issuecomment-902332088) in the original PR. Given that #21245 has received a lot of review*, I have decided to open this new pull request with [modifications required to get ACK from luke-jr ](https://github.com/bitcoin/bitcoin/pull/21245#issuecomment-905150806) and a few nits of mine.
### Original PR description
> Display the prevout in transaction inputs when calling getblock level 3 verbosity. This PR affects the existing `/rest/block` API by adding a `prevout` fields to tx inputs. This is mentioned in the change to the release notes.
>
> I added some functional tests that
>
> * checks that the RPC call still works when TxUndo can't be found
>
> * Doesn't display the "value" or "scriptPubKey" of the previous output when at a lower verbosity level
>
>
> This "completes" the issue #18771
### Possible improvements
* https://github.com/kiminuo/bitcoin/commit/b0bf4f255f86aeaddce68889087c22f9068f4d97 - I can include even this commit to this PR if deemed useful or I can leave it for a follow-up PR. See https://github.com/bitcoin/bitcoin/pull/21245#issuecomment-894853784 for more context.
### Examples
Examples of the `getblock` output with various verbose levels. Note that `000000000000001f682b188971cc1a121546be4e9d5baf22934fdc7f538288d5` contains only 2 transactions.
#### Verbose level 0
```bash
./bitcoin-cli -testnet getblock 000000000000001f682b188971cc1a121546be4e9d5baf22934fdc7f538288d5 0
```
##### Verbose level 1
```bash
./bitcoin-cli -testnet getblock 000000000000001f682b188971cc1a121546be4e9d5baf22934fdc7f538288d5 1
```
##### Verbose level 2
```bash
./bitcoin-cli -testnet getblock 000000000000001f682b188971cc1a121546be4e9d5baf22934fdc7f538288d5 2
```
##### Verbose level 3
```bash
./bitcoin-cli -testnet getblock 000000000000001f682b188971cc1a121546be4e9d5baf22934fdc7f538288d5 3
```
#### REST
```bash
curl -H "content-type:text/plain;" http://127.0.0.1:18332/rest/block/000000000000001f682b188971cc1a121546be4e9d5baf22934fdc7f538288d5.json
```
<sub>* ... and my everyday obsessive checking of my email inbox whether the PR moves forward.</sub>
Edit laanwj: Removed at symbol from message, and large example output to prevent it from all ending up in the commit message.
ACKs for top commit:
0xB10C:
ACK 5c34507ecbbdc29c086276d1c62835b461823507
meshcollider:
utACK 5c34507ecbbdc29c086276d1c62835b461823507
theStack:
ACK 5c34507ecbbdc29c086276d1c62835b461823507 👘
promag:
Concept ACK 5c34507ecbbdc29c086276d1c62835b461823507
Tree-SHA512: bbff120d8fd76e617b723b102b0c606e0d8eb27f21c631d5f4cdab0892137c4bc7c65b1df144993405f942c91be47a26e80480102af55bff22621c19f518aea3
|
|
This adds a new rest endpoint:
/rest/blockfilter/filtertype/requesttype/blockhash (eg
/rest/blockfilter/basic/header/000000000019d6689c085ae165831e934ff763ae46a2a6c172b3f1b60a8ce26f.hex)
which exposes either the filter "header" or the filter data itself.
Most of the code is cribbed from the equivalent RPC.
|
|
|
|
Display the prevout in transaction inputs when calling getblock level 3
verbosity.
Co-authored-by: Luke Dashjr <luke_github1@dashjr.org>
Co-authored-by: 0xB10C <19157360+0xB10C@users.noreply.github.com>
|
|
Co-authored-by: Luke Dashjr <luke_github1@dashjr.org>
Co-authored-by: 0xB10C <19157360+0xB10C@users.noreply.github.com>
|
|
|
|
|
|
This is not the cleanest change but:
1. It fixes the erroneous use of RPC's Ensure*() in rest.cpp, which
cause crashes in REST contexts.
RPC code wraps all calls in a try/except, REST code does not.
Ensure*(), being part of RPC, expects that its throw's will get
caught by a try/except. But if you use Ensure*() in REST code, since
it doesn't have a try/except wrap, a crash will happen.
2. It is consistent with other functions like GetMemPool.
Someone can probably make this a bit prettier.
|
|
586190f0b4740457cb86cba632e3d64e6dfe9b0c rpc/rest: Take and reuse local Chain/ChainState obj (Carl Dong)
bc3bd369027273278a0541f3b991eb71de831aa2 rpc: style: Improve BuriedForkDescPushBack signature (Carl Dong)
f99913969f92b8b9cef1b83f5ee8e6a9267b4af0 rpc: Remove unnecessary casting of block height (Carl Dong)
6a3d1920209cded0dae52fb9070a3530d9a4e5fd rpc: Tidy up local references (see commit message) (Carl Dong)
038854f31e3511e8bb6e163305cab0a96783d25b rest/rpc: Remove now-unused old Ensure functions (Carl Dong)
6fb65b49f4d393b091479be5a5df5a0a160cf986 scripted-diff: rest/rpc: Use renamed EnsureAny*() (Carl Dong)
1570c7ee98612366df031bebef9e0468fb57b8a2 rpc: Add renamed EnsureAny*() functions (Carl Dong)
306b1cd3eeb2502904ed4698646d2c86d028aad2 rpc: Add alt Ensure* functions acepting NodeContext (Carl Dong)
d7824acdb9b18fe8f151771a83ccae1681f16c66 rest: Use existing NodeContext (Carl Dong)
3f0893479908ca28d6127c8d0ada30737cb830be rest: Pass in NodeContext to rest_block (Carl Dong)
7be0671b950842fc3a17641a4a21501de0a800b5 rpc/rawtx: Use existing NodeContext (Carl Dong)
60dc05afc6f6388c6f86729a0edd7cb69f1748e0 rpc/mining: Use existing NodeContext (Carl Dong)
d485e815e2b62dc74a485569d08130dc3ef9ff63 rpc/blockchain: Use existing NodeContext (Carl Dong)
d0abf0bf429586e3a5b4c3231fe430dc29695481 rpc/*,rest: Add review-only assertion to EnsureChainman (Carl Dong)
cced0f46c9133e0fc6211e987421ad1d9be1a399 miner: Pass in previous CBlockIndex to RegenerateCommitments (Carl Dong)
Pull request description:
Overall PR: #20158 (tree-wide: De-globalize ChainstateManager)
Based on:
- [x] #21270 | [Bundle 4/n] Prune g_chainman usage in validation-adjacent modules
- [x] #21525 | [Bundle 4.5/n] Followup fixups to bundle 4
Note to reviewers:
1. This bundle may _apparently_ introduce usage of `g_chainman` or `::Chain(state|)Active()` globals, but these are resolved later on in the overall PR. [Commits of overall PR](https://github.com/bitcoin/bitcoin/pull/20158/commits)
2. There may be seemingly obvious local references to `ChainstateManager` or other validation objects which are not being used in callers of the current function in question, this is done intentionally to **_keep each commit centered around one function/method_** to ease review and to make the overall change systematic. We don't assume anything about our callers. Rest assured that once we are considering that particular caller in later commits, we will use the obvious local references. [Commits of overall PR](https://github.com/bitcoin/bitcoin/pull/20158/commits)
3. When changing a function/method that has many callers (e.g. `LookupBlockIndex` with 55 callers), it is sometimes easier (and less error-prone) to use a scripted-diff. When doing so, there will be 3 commits in sequence so that every commit compiles like so:
1. Add `new_function`, make `old_function` a wrapper of `new_function`, divert all calls to `old_function` to `new_function` **in the local module only**
2. Scripted-diff to divert all calls to `old_function` to `new_function` **in the rest of the codebase**
3. Remove `old_function`
ACKs for top commit:
ryanofsky:
Code review ACK 586190f0b4740457cb86cba632e3d64e6dfe9b0c. Since last review, no changes to existing commits, just some simple new commits added: three new commits renaming std::any Ensure functions (scripted diff commit and manual pre/post commits), and one new commit factoring out a repeated `ActiveChain()` call made in a loop. Thanks for the updates!
jnewbery:
utACK 586190f0b4740457cb86cba632e3d64e6dfe9b0c
MarcoFalke:
review ACK 586190f0b4740457cb86cba632e3d64e6dfe9b0c 🍯
Tree-SHA512: 64b677fb50141805b55c3f1afe68fcd298f9a071a359bdcd63256d52e334f83e462f31fb3ebee9b630da8f1d912a03a128cfc38179e7aaec29a055744a98478c
|
|
In all rest/rpc-related modules, if there are multiple calls to
ActiveChain{,State}(), and the calls fall under the same ::cs_main lock,
we can simply take a local reference and use/reuse it instead of calling
ActiveChain{,State}() again and again.
|
|
Organize local variables/references such that:
1. There is always a `ChainstateManager` reference before any `LOCK(cs_main)`.
2. NodeContext references are used with Ensure*() functions introduced in
previous commit where appropriate to avoid duplicate assertions.
|
|
-BEGIN VERIFY SCRIPT-
sed -i -E 's@Ensure([^(]+)(\((request\.|)context\))@EnsureAny\1\2@g' \
-- src/rest.cpp src/rpc/*.cpp
-END VERIFY SCRIPT-
|
|
fadcd3f78e1dd1acd7a774f8fad68dc471ff9e1f doc: Remove irrelevant link to GitHub (MarcoFalke)
fa121b628d51bb0e25eb3fbd716881fa55527dc7 blockstorage: [refactor] Use chainman reference where possible (MarcoFalke)
fa0c7d9ad24d3c9515d3f9c136af4071cbd79055 move-only: Move *Disk functions to blockstorage (MarcoFalke)
fa91b2b2b3447a3645e7958c7dc4e1946a69cb9c move-only: Move AbortNode to shutdown (MarcoFalke)
fa413f07a14744e7d7f7746e861aabd9cf938f61 move-only: Move ThreadImport to blockstorage (MarcoFalke)
faf843c07f99f91603e08ea858f972516f1d669a refactor: Move load block thread into ChainstateManager (MarcoFalke)
Pull request description:
This picks up the closed pull request #21030 and is the first step toward fixing #21220.
The basic idea is to move all disk access into a separate module with benefits:
* Breaking down the massive files init.cpp and validation.cpp into logical units
* Creating a standalone-module to reduce the mental complexity
* Pave the way to fix validation related circular dependencies
* Pave the way to mock disk access for testing, especially where it is performance critical (like fuzzing)
ACKs for top commit:
promag:
Code review ACK fadcd3f78e, checked (almost) moved only changes. This is a nice tidy up change and doesn't change behavior. Easily reviewed commit by commit.
jamesob:
ACK fadcd3f78e1dd1acd7a774f8fad68dc471ff9e1f ([`jamesob/ackr/21575.1.MarcoFalke.refactor_create_blocksto`](https://github.com/jamesob/bitcoin/tree/ackr/21575.1.MarcoFalke.refactor_create_blocksto))
ryanofsky:
Code review ACK fadcd3f78e1dd1acd7a774f8fad68dc471ff9e1f. New organization makes sense, moves extraneous things outside of validation.cpp. PR is also easy to review with helpfully split up moveonly commits.
Tree-SHA512: 917996592b6d8f9998289d8cb2b1b78b23d1fdb3b07216c9caec1380df33baa09dc2c1e706da669d440b497e79c9c62a01ca20dc202df5ad974a75f3ef7a143b
|
|
This just makes an additional simplification after #21366 replaced
util::Ref with std::any. It was originally suggested
https://github.com/bitcoin/bitcoin/pull/21366#issuecomment-792044351 but
delayed for a followup. It would have prevented usage bug
https://github.com/bitcoin/bitcoin/pull/21572.
|
|
Can be reviewed with the git options
--color-moved=dimmed-zebra --color-moved-ws=ignore-all-space
|
|
|
|
|
|
|
|
e829c9afbf75e930db6c3fe77a269b0af5e7a3ad refactor: replace sizeof(a)/sizeof(a[0]) by std::size (C++17) (Sebastian Falbesoner)
365539c84691d470b44d35df374d8c049f8c1192 refactor: init vectors via std::{begin,end} to avoid pointer arithmetic (Sebastian Falbesoner)
63d4ee1968144cc3d115f92baef95785abf813ac refactor: iterate arrays via C++11 range-based for loops if idx is not needed (Sebastian Falbesoner)
Pull request description:
This refactoring PR picks up the idea of #19626 and replaces all occurences of `sizeof(x)/sizeof(x[0])` (or `sizeof(x)/sizeof(*x)`, respectively) with the now-available C++17 [`std::size`](https://en.cppreference.com/w/cpp/iterator/size) (as [suggested by sipa](https://github.com/bitcoin/bitcoin/pull/19626#issuecomment-666487228)), making the macro `ARRAYLEN` obsolete.
As preparation for this, two other changes are done to eliminate `sizeof(x)/sizeof(x[0])` usage:
* all places where arrays are iterated via an index are changed to use C++11 range-based for loops If the index' only purpose is to access the array element (as [suggested by MarcoFalke](https://github.com/bitcoin/bitcoin/pull/19626#discussion_r463404541)).
* `std::vector` initializations are done via `std::begin` and `std::end` rather than using pointer arithmetic to calculate the end (also [suggested by MarcoFalke](https://github.com/bitcoin/bitcoin/pull/20429#discussion_r567418821)).
ACKs for top commit:
practicalswift:
cr ACK e829c9afbf75e930db6c3fe77a269b0af5e7a3ad: patch looks correct
fanquake:
ACK e829c9afbf75e930db6c3fe77a269b0af5e7a3ad
MarcoFalke:
review ACK e829c9afbf75e930db6c3fe77a269b0af5e7a3ad 🌩
Tree-SHA512: b01d32c04b9e04d562b7717cae00a651ec9a718645047a90761be6959e0cc2adbd67494e058fe894641076711bb09c3b47a047d0275c736f0b2218e1ce0d193d
|
|
|
|
[META] In a previous commit, we moved ::LookupBlockIndex to become a
member function of BlockManager. This commit is split out from
that one since it can be expressed nicely as a scripted-diff.
-BEGIN VERIFY SCRIPT-
find_regex='LookupBlockIndex' \
&& git grep -l -E "$find_regex" -- src \
| grep -v '^src/validation\.\(cpp\|h\)$' \
| xargs sed -i -E "s@${find_regex}@g_chainman.m_blockman.LookupBlockIndex@g"
-END VERIFY SCRIPT-
|
|
-BEGIN VERIFY SCRIPT-
./contrib/devtools/copyright_header.py update ./
-END VERIFY SCRIPT-
|
|
(blockchain,rawtransaction)
fa6bb0ce5dba33970e2c1e47ea4d0d2c0718eccb Assert that RPCArg names are equal to CRPCCommand ones (rawtransaction) (MarcoFalke)
fa80c814874a2893e4323ba5148fba21d7f421cd Assert that RPCArg names are equal to CRPCCommand ones (blockchain) (MarcoFalke)
Pull request description:
This is split out from #18531 to just touch some RPC methods. Description from the main pr:
### Motivation
RPCArg names in the rpc help are currently only used for documentation. However, in the future they could be used to teach the server the named arguments. Named arguments are currently registered by the `CRPCCommand`s and duplicate the RPCArg names from the documentation. This redundancy is fragile, and has lead to errors in the past (despite having linters to catch those kind of errors). See section "bugs found" for a list of bugs that have been found as a result of the changes here.
### Changes
The changes here add an assert in the `CRPCCommand` constructor that the RPCArg names are identical to the ones in the `CRPCCommand`.
### Future work
> Here or follow up, makes sense to also assert type of returned UniValue?
Sure, but let's not get ahead of ourselves. I am going to submit any further works as follow-ups, including:
* Removing the CRPCCommand arguments, now that they are asserted to be equal and thus redundant
* Removing all python regex linters on the args, now that RPCMan can be used to generate any output, including the cli.cpp table
* Auto-formatting and sanity checking the RPCExamples with RPCMan
* Checking passed-in json in self-check. Removing redundant checks
* Checking returned json against documentation to avoid regressions or false documentation
* Compile the RPC documentation at compile-time to ensure it doesn't change at runtime and is completely static
### Bugs found
* The assert identified issue #18607
* The changes itself fixed bug #19250
ACKs for top commit:
fjahr:
utACK fa6bb0ce5dba33970e2c1e47ea4d0d2c0718eccb
tryphe:
utACK fa6bb0ce5dba33970e2c1e47ea4d0d2c0718eccb. Reducing data duplication is nice. Code changes are minimal and concise.
Tree-SHA512: deb0edc3f999baf055526eaa199b98c500635e12502dece7aa3cad5319db330eb5ee7459a5c8f040a83671a7f20c560c19a2026fb76c8416f138aa332727cbce
|
|
|
|
|
|
|
|
HexStr can be called with anything that bas `begin()` and `end()` functions,
so clean up the redundant calls.
|
|
b3f7f375efb9a9ca9a7a4f2caf41fe3df2262520 refactor: Remove g_rpc_node global (Russell Yanofsky)
ccb5059ee89f6e8dc31ba5b82830b384890bb65e scripted-diff: Remove g_rpc_node references (Russell Yanofsky)
6fca33b2edc09ed62dab2323c780b31585de1750 refactor: Pass NodeContext to RPC and REST methods through util::Ref (Russell Yanofsky)
691c817b340d10e806dc3b1834d2a8fcc5e681fd Add util::Ref class as temporary alternative for c++17 std::any (Russell Yanofsky)
Pull request description:
This PR removes the `g_rpc_node` global, to get same benefits we see removing other globals and make RPC code more testable, modular, and reusable.
This uses a hybrid of the approaches suggested in #17548. Instead of using `std::any`, which isn't available in c++11, or `void*`, which isn't type safe, it uses a small new `util::Ref` helper class, which acts like a simplified `std::any` that only holds references, not values.
Motivation for writing this was to provide an simpler alternative to #18647 by Harris Brakmić (brakmic) which avoids some shortcomings of that PR (https://github.com/bitcoin/bitcoin/pull/18647#issuecomment-617878826)
ACKs for top commit:
MarcoFalke:
re-ACK b3f7f375ef, only change is adding back const and more tests 🚾
ajtowns:
ACK b3f7f375efb9a9ca9a7a4f2caf41fe3df2262520
Tree-SHA512: 56292268a001bdbe34d641db1180c215351503966ff451e55cc96c9137f1d262225d7d7733de9c9da7ce7d7a4b34213a98c2476266b58c89dbbb0f3cb5aa5d70
|
|
f9ee0f37c28f604bc82dab502ce229c66ef5b3b9 Add comments to CustomUintFormatter (Pieter Wuille)
4eb5643e3538863c9d2ff261f49a9a1b248de243 Convert everything except wallet/qt to new serialization (Pieter Wuille)
2b1f85e8c52c8bc5a17eae4c809eaf61d724af98 Convert blockencodings_tests to new serialization (Pieter Wuille)
73747afbbeb013669faf4c4d2c0903cec4526fb0 Convert merkleblock to new serialization (Pieter Wuille)
d06fedd1bc26bf5bf2b203d4445aeaebccca780e Add SER_READ and SER_WRITE for read/write-dependent statements (Russell Yanofsky)
6f9a1e5ad0a270d3b5a715f3e3ea0911193bf244 Extend CustomUintFormatter to support enums (Russell Yanofsky)
769ee5fa0011ae658770586442715452a656559d Merge BigEndian functionality into CustomUintFormatter (Pieter Wuille)
Pull request description:
The next step of changes from #10785.
This:
* Adds support for enum serialization to `CustomUintFormatter`, used in `CAddress` for service flags.
* Merges `BigEndian` into `CustomUintFormatter`, used in `CNetAddr` for port numbers.
* Converts everything (except wallet and gui) to use the new serialization framework.
ACKs for top commit:
MarcoFalke:
re-ACK f9ee0f37c2, only change is new documentation commit for CustomUintFormatter 📂
ryanofsky:
Code review ACK f9ee0f37c28f604bc82dab502ce229c66ef5b3b9. Just new commit adding comment since last review
jonatack:
Code review re-ACK f9ee0f37c28f604bc82dab502ce229c6 only change since last review is an additional commit adding Doxygen documentation for `CustomUintFormatter`.
Tree-SHA512: e7a0a36afae592d5a4ff8c81ae04d858ac409388e361f2bc197d9a78abca45134218497ab2dfd6d031e0cce0ca586cf857077b7c6ce17fccf67e2d367c1b6cd4
|
|
This commit does not change behavior
|
|
ff6549c3c84ca7324032dbc37744645bf2fe1c3e fix: update rest info on block size and json (Chris Abrams)
Pull request description:
Addressing the ambiguous block size text in rest docs: https://github.com/bitcoin/bitcoin/issues/18703
Also makes sure to let developers know there is `.json` option for the rest output format.
ACKs for top commit:
MarcoFalke:
ACK ff6549c3c84ca7324032dbc37744645bf2fe1c3e
promag:
ACK ff6549c3c84ca7324032dbc37744645bf2fe1c3e.
Tree-SHA512: 9ef93c1432d650b1f9599778ba092c1ca5b084a537af257078e1c713c76c5d3a4cc4b1ede8a2489964be8ed0303ad8bea58c1cb4759bbb9b24dbdebfec8001d3
|
|
|
|
A bad interaction between valgrind and clang 6.0.0-1ubuntu2 with -O2
optimizations makes valgrind misleadingly imply C++ code is reading an
uninitialized blockheight value in rest_blockhash_by_height just because that's
what clang optimized code is doing. The C++ code looks like:
int32_t blockheight;
if (!ParseInt32(height_str, &blockheight) || blockheight < 0) {
while the optimized code looks like:
0x00000000000f97ab <+123>: callq 0x4f8860 <ParseInt32(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, int*)>
0x00000000000f97b0 <+128>: mov 0xc(%rsp),%ebx
0x00000000000f97b4 <+132>: test %ebx,%ebx
0x00000000000f97b6 <+134>: js 0xf98aa <rest_blockhash_by_height(util::Ref const&, HTTPRequest*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)+378>
0x00000000000f97bc <+140>: xor $0x1,%al
0x00000000000f97be <+142>: jne 0xf98aa <rest_blockhash_by_height(util::Ref const&, HTTPRequest*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)+378>
During the rest_interface.py test:
self.test_rest_request("/blockhashbyheight/", ret_type=RetType.OBJ, status=400)
when height_str is empty, ParseInt32 returns false and blockheight value is
never assigned. The optimized code reads the uninitialized blockheight value
in 0xc(%rsp) before the checking the ParseInt32 return value in %al, which is
harmless, but triggers the following error from valgrind:
==30660== Thread 13 b-httpworker.2:
==30660== Conditional jump or move depends on uninitialised value(s)
==30660== at 0x2017B6: rest_blockhash_by_height(util::Ref const&, HTTPRequest*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (rest.cpp:614)
==30660== by 0x2041B9: operator() (rest.cpp:670)
==30660== by 0x2041B9: std::_Function_handler<bool (HTTPRequest*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&), StartREST(util::Ref const&)::$_1>::_M_invoke(std::_Any_data const&, HTTPRequest*&&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (std_function.h:301)
==30660== by 0x3EC994: operator() (std_function.h:706)
==30660== by 0x3EC994: HTTPWorkItem::operator()() (httpserver.cpp:55)
==30660== by 0x3ED16D: WorkQueue<HTTPClosure>::Run() (httpserver.cpp:114)
==30660== by 0x3E9168: HTTPWorkQueueRun(WorkQueue<HTTPClosure>*, int) (httpserver.cpp:342)
==30660== by 0x3EDAAA: __invoke_impl<void, void (*)(WorkQueue<HTTPClosure> *, int), WorkQueue<HTTPClosure> *, int> (invoke.h:60)
==30660== by 0x3EDAAA: __invoke<void (*)(WorkQueue<HTTPClosure> *, int), WorkQueue<HTTPClosure> *, int> (invoke.h:95)
==30660== by 0x3EDAAA: _M_invoke<0, 1, 2> (thread:234)
==30660== by 0x3EDAAA: operator() (thread:243)
==30660== by 0x3EDAAA: std::thread::_State_impl<std::thread::_Invoker<std::tuple<void (*)(WorkQueue<HTTPClosure>*, int), WorkQueue<HTTPClosure>*, int> > >::_M_run() (thread:186)
==30660== by 0x64256DE: ??? (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.25)
==30660== by 0x54876DA: start_thread (pthread_create.c:463)
==30660== by 0x6DC888E: clone (clone.S:95)
==30660== Uninitialised value was created by a stack allocation
==30660== at 0x20173A: rest_blockhash_by_height(util::Ref const&, HTTPRequest*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (rest.cpp:608)
==30660==
{
<insert_a_suppression_name_here>
Memcheck:Cond
fun:_ZL24rest_blockhash_by_heightRKN4util3RefEP11HTTPRequestRKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE
fun:operator()
fun:_ZNSt17_Function_handlerIFbP11HTTPRequestRKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEEZ9StartRESTRKN4util3RefEE3$_1E9_M_invokeERKSt9_Any_dataOS1_S9_
fun:operator()
fun:_ZN12HTTPWorkItemclEv
fun:_ZN9WorkQueueI11HTTPClosureE3RunEv
fun:_ZL16HTTPWorkQueueRunP9WorkQueueI11HTTPClosureEi
fun:__invoke_impl<void, void (*)(WorkQueue<HTTPClosure> *, int), WorkQueue<HTTPClosure> *, int>
fun:__invoke<void (*)(WorkQueue<HTTPClosure> *, int), WorkQueue<HTTPClosure> *, int>
fun:_M_invoke<0, 1, 2>
fun:operator()
fun:_ZNSt6thread11_State_implINS_8_InvokerISt5tupleIJPFvP9WorkQueueI11HTTPClosureEiES6_iEEEEE6_M_runEv
obj:/usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.25
fun:start_thread
fun:clone
}
This is a known bad interaction between clang and valgrind. The clang optimized
code is correct but valgrind has no way of knowing that accessing the
uninitialized value isn't a problem. Issue has been reported previously:
https://bugs.llvm.org/show_bug.cgi?id=32604#c4
https://github.com/Z3Prover/z3/issues/972
This commit just sets blockheight to 0 as a workaround.
|
|
|
|
-BEGIN VERIFY SCRIPT-
./contrib/devtools/copyright_header.py update ./
-END VERIFY SCRIPT-
|
|
|
|
|
|
This aliasing makes subsequent commits easier to review; eventually CoinsTip()
will return the CCoinsViewCache managed by CChainState.
|
|
|