aboutsummaryrefslogtreecommitdiff
path: root/src/consensus
AgeCommit message (Collapse)Author
2023-11-16Include version.h in fewer placesAnthony Towns
2023-11-14Use ParamsWrapper for witness serializationAnthony Towns
2023-11-06[validation] add TxValidationResult::TX_RECONSIDERABLE, TX_UNKNOWNglozow
With package validation rules, transactions that fail individually may sometimes be eligible for reconsideration if submitted as part of a (different) package. For now, that includes trasactions that failed for being too low feerate. Add a new TxValidationResult type to distinguish these failures from others. In the next commits, we will abort package validation if a tx fails for any other reason. In the future, we will also decide whether to cache failures in recent_rejects based on this result (we won't want to reject a package containing a transaction that was rejected previously for being low feerate). Package validation also sometimes elects to skip some transactions when it knows the package will not be submitted in order to quit sooner. Add a result to specify this situation; we also don't want to cache these as rejections.
2023-09-08consensus/validation.h: remove needless GetTransactionOutputWeight helperAntoine Poinsot
Introduced in 9b7ec393b82ca9d7ada77d06e0835df0386a8b85. This copied the format of the other Get.*Weight helpers but it's useless for a CTxOut.
2023-08-25wallet: use descriptor satisfaction size to estimate inputs sizeAntoine Poinsot
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.
2023-06-12Use `int32_t` type for most transaction size/weight valuesHennadii Stepanov
This change gets rid of a few casts and makes the following commit diff smaller.
2023-02-06Merge bitcoin/bitcoin#26345: refactor: modernize the implementation of uint256.*Andrew Chow
935acdcc79d1dc5ac04a83b92e5919ddbfa29329 refactor: modernize the implementation of uint256.* (pasta) Pull request description: - Constructors of uint256 to utilize Span instead of requiring a std::vector - converts m_data into a std::array - Prefers using `WIDTH` instead of `sizeof(m_data)` - make all the things constexpr - replace C style functions with c++ equivalents - memset -> std::fill This may also be replaced by std::memset, but I think that std::fill is more idiomatic of modern c++ and readable. - memcpy -> std::copy Note: In practice, implementations of std::copy avoid multiple assignments and use bulk copy functions such as std::memmove if the value type is TriviallyCopyable and the iterator types satisfy LegacyContiguousIterator. (https://en.cppreference.com/w/cpp/algorithm/copy) This could also likely be replaced by std::memcpy, but as said above, I believe the using std::copy is the more c++ way to do anything and is almost guaranteed to compile to the same asm - memcmp -> std::memcmp ACKs for top commit: achow101: ACK 935acdcc79d1dc5ac04a83b92e5919ddbfa29329 hebasto: Approach ACK 935acdcc79d1dc5ac04a83b92e5919ddbfa29329. aureleoules: reACK 935acdcc79d1dc5ac04a83b92e5919ddbfa29329 john-moffett: ACK 935acdcc79d1dc5ac04a83b92e5919ddbfa29329 stickies-v: Approach ACK 935acdcc7 Tree-SHA512: 4f1ba54ff2198eea0e505d41e73d552c84c60f6878d5c85a94a8ab57f39afc94ef8d79258e7afd01fa84ec2a99f4404bb877eecd671f65e1ee9273f3129fc650
2023-01-04Merge bitcoin/bitcoin#26603: doc: CalculateSequenceLocks: prevHeights ↵glozow
entries are set to 0, not removed f537127271b1f22ee4651915b7b9266e0df72841 doc: fix: prevHeights entries are set to 0, not removed (stickies-v) Pull request description: In [`CalculateSequenceLocks`](https://github.com/bitcoin/bitcoin/blob/a035b6a0c418d0b720707df69559028bd662fa70/src/consensus/tx_verify.h#L69) no items are removed from `prevHeights`, they are just set to 0: https://github.com/bitcoin/bitcoin/blob/a035b6a0c418d0b720707df69559028bd662fa70/src/consensus/tx_verify.cpp#L69-L73 This PR updates the docs to reflect the actual implementation. Seems to have been wrongly documented since introduction in #7184 already ([implementation](https://github.com/bitcoin/bitcoin/pull/7184/files#diff-34d21af3c614ea3cee120df276c9c4ae95053830d7f1d3deaf009a4625409ad2R742-R749) and [documentation](https://github.com/bitcoin/bitcoin/pull/7184/files#diff-34d21af3c614ea3cee120df276c9c4ae95053830d7f1d3deaf009a4625409ad2R712-R713)) ACKs for top commit: hebasto: ACK f537127271b1f22ee4651915b7b9266e0df72841 Tree-SHA512: 3661501660f6832b2116fd83466ffe95a60b341c14cb09a37489e2a587bea3290b0528690120a0f644c3eea02177aa1fb8968258482fa43b0303e016abb17418
2022-12-24scripted-diff: Bump copyright headersHennadii Stepanov
-BEGIN VERIFY SCRIPT- ./contrib/devtools/copyright_header.py update ./ -END VERIFY SCRIPT- Commits of previous years: - 2021: f47dda2c58b5d8d623e0e7ff4e74bc352dfa83d7 - 2020: fa0074e2d82928016a43ca408717154a1c70a4db - 2019: aaaaad6ac95b402fe18d019d67897ced6b316ee0
2022-12-10refactor: modernize the implementation of uint256.*pasta
- Constructors of uint256 to utilize Span instead of requiring a std::vector - converts m_data into a std::array - Prefers using `WIDTH` instead of `sizeof(m_data)` - make all the things constexpr - replace C style functions with c++ equivalents - memset -> std::fill - memcpy -> std::copy Note: In practice, implementations of std::copy avoid multiple assignments and use bulk copy functions such as std::memmove if the value type is TriviallyCopyable and the iterator types satisfy LegacyContiguousIterator. (https://en.cppreference.com/w/cpp/algorithm/copy) - memcmp -> std::memcmp
2022-11-29doc: fix: prevHeights entries are set to 0, not removedstickies-v
2022-09-23refactor: use <cstdlib> over stdlib.hfanquake
We currently use both. Consolidate on the former.
2022-08-29Require callers of AcceptBlockHeader() to perform anti-dos checksSuhas Daftuar
In order to prevent memory DoS, we must ensure that we don't accept a new header into memory until we've performed anti-DoS checks, such as verifying that the header is part of a sufficiently high work chain. This commit adds a new argument to AcceptBlockHeader() so that we can ensure that all call-sites which might cause a new header to be accepted into memory have to grapple with the question of whether the header is safe to accept, or needs further validation. This patch also fixes two places where low-difficulty-headers could have been processed without such validation (processing an unrequested block from the network, and processing a compact block). Credit to Niklas Gögge for noticing this issue, and thanks to Sjors Provoost for test code.
2022-08-05Add time helpersMacroFake
To be used in the next commit
2022-06-22Remove LOCKTIME_MEDIAN_TIME_PAST constantMarcoFalke
2022-05-26Merge bitcoin/bitcoin#24032: Add defaults to vDeployments to avoid ↵laanwj
uninitialized variables c4c5b9ca6e98cf44309af73edf5559940a04d00f consensus/params: set default values for BIP9Deployment (Anthony Towns) Pull request description: Adds default values for `vDeployments` in `consensus/params.h` so that undefined behaviour is avoided if a deployment is not initialized. Also adds a check in the unit tests to alert if this is happening, since even if it doesn't invoke undefined behaviour it's probably a mistake. ACKs for top commit: laanwj: Code review ACK c4c5b9ca6e98cf44309af73edf5559940a04d00f Tree-SHA512: 22d7ff86a817d9e9e67c47301fc3b7e9d5821c13565d7706711f113dea220eea29b413a7c8d029691009159cebc85a108d77cb52418734091c196bafb2b12423
2022-05-05Sanity assert GetAncestor() != nullptr where appropriateAdam Jonas
Add sanity asserts for return value of `CBlockIndex::GetAncestor()` where appropriate. In validation.cpp `CheckSequenceLocks`, check the return value of `tip->GetAncestor(maxInputHeight)` stored into `lp->maxInputBlock`. If it ever returns `nullptr` because the ancestor isn't found, it's going to be a bad bug to keep going, since a `LockPoints` object with the `maxInputBlock` member set to `nullptr` signifies no relative lock time. In the other places, the added asserts would prevent accidental dereferencing of a null pointer which is undefined behavior. Co-Authored-By: Aurèle Oulès <aurele@oules.com> Co-Authored-By: danra <danra@users.noreply.github.com>
2022-04-05consensus/params: set default values for BIP9DeploymentAnthony Towns
While chainparams should explicilty set values for each possible entry in vDeployments, in the past that has been missed resulting in potential undefined behaviour due to accessing unitinitialized data. Reduce the severity of future bugs of that nature by providing benign default values. Adds a unit test to alert if the default value is not overwritten for the real chains (NEVER_ACTIVE/NEVER_ACTIVE rather than NEVER_ACTIVE/NO_TIMEOUT).
2022-03-25Merge bitcoin/bitcoin#23536: Enforce Taproot script flags whenever WITNESS ↵laanwj
is set cccc1e70b8a14430cc94143da97936a60d6c83d3 Enforce Taproot script flags whenever WITNESS is set (MarcoFalke) fa422994116a7a053789304d56159760081479eb Remove nullptr check in GetBlockScriptFlags (MarcoFalke) faadc606c7644f2934de390e261d9d65a81a7592 refactor: Pass const reference instead of pointer to GetBlockScriptFlags (MarcoFalke) Pull request description: Now that Taproot is active, it makes sense to enforce its rules on all blocks, even historic ones, regardless of the deployment status. ### Benefits: (With "script flags" I mean "taproot script verification flags".) * Script flags are known ahead for all blocks (even blocks not yet created) and do not change. This may benefit static analysis, code review, and development of new script features that build on Taproot. * Any future bugs introduced in the deployment code won't have any effect on the script flags, as they are independent of deployment. * Enforcing the taproot rules regardless of the deployment status makes testing easier because invalid blocks after activation are also invalid before activation. So there is no need to differentiate the two cases. * It gives belt-and-suspenders protection against a practically expensive and theoretically impossible IBD reorg attack where the node is eclipsed. While `nMinimumChainWork` already protects against this, the cost for a few months worth of POW might be lowered until a major version release of Bitcoin Core reaches EOL. The needed work for the attack is the difference between `nMinimumChainWork` and the work at block 709632. For reference, previously the same was done for P2SH and WITNESS in commit 0a8b7b4b33c9d78574627fc606267e2d8955cd1c. ### Implementation: I found one block which fails verification with the flags applied, so I added a `TaprootException`, similar to the `BIP16Exception`. For reference, the debug log: ``` ERROR: ConnectBlock(): CheckInputScripts on b10c007c60e14f9d087e0291d4d0c7869697c6681d979c6639dbd960792b4d41 failed with non-mandatory-script-verify-flag (Witness program was passed an empty witness) BlockChecked: block hash=0000000000000000000f14c35b2d841e986ab5441de8c585d5ffe55ea1e395ad state=non-mandatory-script-verify-flag (Witness program was passed an empty witness) InvalidChainFound: invalid block=0000000000000000000f14c35b2d841e986ab5441de8c585d5ffe55ea1e395ad height=692261 log2_work=92.988459 date=2021-07-23T08:24:20Z InvalidChainFound: current best=0000000000000000000067b17a4c0ffd77c29941b15ad356ca8f980af137a25d height=692260 log2_work=92.988450 date=2021-07-23T07:47:31Z ERROR: ConnectTip: ConnectBlock 0000000000000000000f14c35b2d841e986ab5441de8c585d5ffe55ea1e395ad failed, non-mandatory-script-verify-flag (Witness program was passed an empty witness) ``` Hint for testing, make sure to set `-noassumevalid`. ### Considerations Obviously this change can lead to consensus splits on the network in light of massive reorgs. Currently the last block before Taproot activation, that is the last block without the Taproot script flags set, is only buried by a few days of POW. However, when and if this patch is included in the next major release, it will be buried by a few months of POW. BIP90 considerations apply when looking at reorgs this large. ACKs for top commit: Sjors: tACK cccc1e70b8a14430cc94143da97936a60d6c83d3 achow101: ACK cccc1e70b8a14430cc94143da97936a60d6c83d3 laanwj: Code review ACK cccc1e70b8a14430cc94143da97936a60d6c83d3 ajtowns: ACK cccc1e70b8a14430cc94143da97936a60d6c83d3 ; code review; wrote a "getblockscriptflags" rpc to quickly check that blocks just had bit 17 (taproot) added; review of earlier revisions had established non-exception blocks do validate with taproot rules enabled. jamesob: ACK cccc1e70b8a14430cc94143da97936a60d6c83d3 ([`jamesob/ackr/23536.1.MarcoFalke.enforce_taproot_script_f`](https://github.com/jamesob/bitcoin/tree/ackr/23536.1.MarcoFalke.enforce_taproot_script_f)) Tree-SHA512: 00044de68939caef6420ffd588c1291c041a8b397c80a3df1e3e3487fbeae1821d23975c51c95e44e774558db76f943b00b4e27cbd0213f64a9253116dc6edde
2022-01-29Enforce Taproot script flags whenever WITNESS is setMarcoFalke
2021-12-30scripted-diff: Bump copyright headersHennadii Stepanov
-BEGIN VERIFY SCRIPT- ./contrib/devtools/copyright_header.py update ./ -END VERIFY SCRIPT- Commits of previous years: * 2020: fa0074e2d82928016a43ca408717154a1c70a4db * 2019: aaaaad6ac95b402fe18d019d67897ced6b316ee0
2021-12-27doc: Remove TODO comment in tx_verifyMarcoFalke
2021-11-16doc: Fix typos in endif header commentsMarcoFalke
2021-11-03[validation] Add CChainState::ProcessTransaction()John Newbery
This just calls through to AcceptToMemoryPool() internally, and is currently unused. Also add a new transaction validation failure reason TX_NO_MEMPOOL to indicate that there is no mempool.
2021-10-16Add comment to COIN constant.Kennan Mell
The COIN constant is critical in understanding Bitcoin's supply, but what it represents isn't clear from the name of the constant. Adding a comment clarifies the meaning of the constant for future readers.
2021-09-30consensus: use <cstdint> over <stdint.h> in amount.hfanquake
2021-09-30consensus: make COIN & MAX_MONEY constexprfanquake
2021-09-30[MOVEONLY] consensus: move amount.h into consensusfanquake
Move amount.h to consensus/amount.h. Renames, adds missing and removes uneeded includes.
2021-08-02consensus/params: simplify ValidDeployment check to avoid gcc warningAnthony Towns
2021-07-20Merge bitcoin/bitcoin#22232: refactor: Pass interpreter flags as uint32_t ↵MarcoFalke
instead of signed int fa621ededdfe31a200b77a8787de7e3d2e667aec refactor: Pass script verify flags as uint32_t (MarcoFalke) Pull request description: The flags are cast to unsigned in the interpreter anyway, so avoid the confusion (and fuzz crashes) by just passing them as unsigned from the beginning. Also, the flags are often inverted bit-wise with the `~` operator, which also works on signed integers, but might cause confusion as the sign bit is flipped. Fixes #22233 ACKs for top commit: theStack: Concept and code review ACK fa621ededdfe31a200b77a8787de7e3d2e667aec kristapsk: ACK fa621ededdfe31a200b77a8787de7e3d2e667aec jonatack: ACK fa621ededdfe31a200b77a8787de7e3d2e667aec Tree-SHA512: ea0720f32f823fa7f075309978672aa39773c6019d12b6c1c9d611fc1983a76115b7fe2a28d50814673bb6415c311ccc05b99d6e871575fb6900faf75ed17769
2021-07-01doc: Move buried deployment doc to the enum that enumerates themMarcoFalke
This is more visible than a comment hidden in an RPC helper function.
2021-07-01Merge bitcoin/bitcoin#19438: Introduce deploymentstatusMarcoFalke
e48826ad87b4f92261f7433e84f48dac9bd9e5c3 tests: remove ComputeBlockVersion shortcut from versionbits tests (Anthony Towns) c5f36725e877d8eb492383844f8ef7535466b366 [refactor] Move ComputeBlockVersion into VersionBitsCache (Anthony Towns) 4a69b4dbe0d7f504811b67c399da7e6d11e4f805 [move-only] Move ComputeBlockVersion from validation to versionbits (Anthony Towns) 0cfd6c6a8f929d5567ac41f95c21548f115efee5 [refactor] versionbits: make VersionBitsCache a full class (Anthony Towns) 8ee3e0bed5bf2cd3c7a68ca6ba6c65f7b9a72cca [refactor] rpc/blockchain.cpp: SoftForkPushBack (Anthony Towns) 92f48f360da5f425428b761219301f509826bec4 deploymentinfo: Add DeploymentName() (Anthony Towns) ea68b3a5729f5d240e968388c4f88acffeb27228 [move-only] Rename versionbitsinfo to deploymentinfo (Anthony Towns) c64b2c6a0f79369624ae96b2e3d579d50aae4de6 scripted-diff: rename versionbitscache (Anthony Towns) de55304f6e7a8b607e6b3fc7436de50910747b0c [refactor] Add versionbits deployments to deploymentstatus.h (Anthony Towns) 2b0d291da8f479739ff394dd92801da8c40b9f8e [refactor] Add deploymentstatus.h (Anthony Towns) eccd736f3dc231ac0306ca763c3b72cf8247230a versionbits: Use dedicated lock instead of cs_main (Anthony Towns) 36a4ba0aaaa9b35185d7178994e36bc02cca9887 versionbits: correct doxygen comments (Anthony Towns) Pull request description: Introduces helper functions to make it easy to bury future deployments, along the lines of the suggestion from [11398](https://github.com/bitcoin/bitcoin/pull/11398#issuecomment-335599326) "I would prefer it if a buried deployment wouldn't require all code paths that check the BIP9 status to require changing". This provides three functions: `DeploymentEnabled()` which tests if a deployment can ever be active, `DeploymentActiveAt()` which checks if a deployment should be enforced in the given block, and `DeploymentActiveAfter()` which checks if a deployment should be enforced in the block following the given block, and overloads all three to work both with buried deployments and versionbits deployments. This adds a dedicated lock for the versionbits cache, which is acquired internally by the versionbits functions, rather than relying on `cs_main`. It also moves moves versionbitscache into deploymentstatus to avoid a circular dependency with validation. ACKs for top commit: jnewbery: ACK e48826ad87b4f92261f7433e84f48dac9bd9e5c3 gruve-p: ACK https://github.com/bitcoin/bitcoin/pull/19438/commits/e48826ad87b4f92261f7433e84f48dac9bd9e5c3 MarcoFalke: re-ACK e48826ad87b4f92261f7433e84f48dac9bd9e5c3 🥈 Tree-SHA512: c846ba64436d36f8180046ad551d8b0d9e20509b9bc185aa2639055fc28803dd8ec2d6771ab337e80da0b40009ad959590d5772f84a0bf6199b65190d4155bed
2021-06-30Merge bitcoin/bitcoin#18096: doc: IsFinalTx comment about nSequence & OP_CLTVMarcoFalke
f9e37f33ce2d8b463a0bcbe7189c9bc5b36530b7 doc: IsFinalTx comment about nSequence & OP_CLTV (Yuval Kogman) Pull request description: It's somewhat surprising that a transaction's `nLockTime` field is ignored when all `nSequence` fields are final, so this change aims to clarify this behavior and cross reference relevant details of `OP_CHECKLOCKTIMEVERIFY`. ACKs for top commit: MarcoFalke: ACK f9e37f33ce2d8b463a0bcbe7189c9bc5b36530b7 Tree-SHA512: 88460dacbe4b8115fb1948715f09b21d4f34ba1da9e88d52f0b774a969f845e9eddc5940e7fee66eacdd3062dc40d6d44c3f282b0e5144411fd47eb2320b44f5
2021-06-30[move-only] Rename versionbitsinfo to deploymentinfoAnthony Towns
2021-06-30[refactor] Add versionbits deployments to deploymentstatus.hAnthony Towns
Adds support for versionbits deployments to DeploymentEnabled, DeploymentActiveAfter and DeploymentActiveAt. Also moves versionbitscache from validation to deploymentstatus.
2021-06-29[refactor] Add deploymentstatus.hAnthony Towns
Provides DeploymentEnabled, DeploymentActiveAt, and DeploymentActiveAfter helpers for checking the status of buried deployments. Can be overloaded so the same syntax works for non-buried deployments, allowing future soft forks to be changed from signalled to buried deployments without having to touch the implementation code. Replaces IsWitnessEnabled and IsScriptWitnessEnabled.
2021-06-14refactor: Pass script verify flags as uint32_tMarcoFalke
They are cast to unsigned anyway when calling VerifyScript, bitcoinconsensus_verify_script*, or CountWitnessSigOps.
2021-05-25Mark `CheckTxInputs` `[[nodiscard]]` (out-param `txfee` only set if call is ↵practicalswift
successful). Avoid UUM in fuzzing harness `coins_view`.
2021-04-27doc: IsFinalTx comment about nSequence & OP_CLTVYuval Kogman
It's somewhat surprising that a transaction's nLockTime field is ignored when all nSequence fields are final, so this change aims to clarify this behavior and cross reference relevant details of OP_CHECKLOCKTIMEVERIFY.
2021-04-12versionbits: Add explicit NEVER_ACTIVE deploymentsAnthony Towns
Previously we used deployments that would timeout prior to Bitcoin's invention, which allowed the deployment to still be activated in unit tests. This switches those deployments to be truly never active.
2021-04-12versionbits: Add support for delayed activationAnthony Towns
2020-12-31scripted-diff: Bump copyright headersMarcoFalke
-BEGIN VERIFY SCRIPT- ./contrib/devtools/copyright_header.py update ./ -END VERIFY SCRIPT-
2020-10-15doc: Move assumed-values doxygen comments to headerMarcoFalke
2020-10-12Activate Taproot/Tapscript on regtest (BIP 341, BIP 342)Pieter Wuille
Define a versionbits-based activation for the new consensus rules on regtest. No activation or activation mechanism is defined for testnet or mainnet.
2020-09-10add signet basic support (signet.cpp)Karl-Johan Alm
Co-authored-by: Anthony Towns <aj@erisian.com.au>
2020-09-10validation: move GetWitnessCommitmentIndex to consensus/validationKarl-Johan Alm
2020-08-04Add txids with non-standard inputs to reject filterSuhas Daftuar
Our policy checks for non-standard inputs depend only on the non-witness portion of a transaction: we look up the scriptPubKey of the input being spent from our UTXO set (which is covered by the input txid), and the p2sh checks only rely on the scriptSig portion of the input. Consequently it's safe to add txids of transactions that fail these checks to the reject filter, as the witness is irrelevant to the failure. This is helpful for any situation where we might request the transaction again via txid (either from txid-relay peers, or if we might fetch the transaction via txid due to parent-fetching of orphans). Further, in preparation for future witness versions being deployed on the network, ensure that WITNESS_UNKNOWN transactions are rejected in AreInputsStandard(), so that transactions spending v1 (or greater) witness outputs will fall into this category of having their txid added to the reject filter.
2020-07-22Merge #18044: Use wtxid for transaction relayWladimir J. van der Laan
0a4f1422cd1c20e12a05d7ff1a2ef1d5e7c654bb Further improve comments around recentRejects (Suhas Daftuar) 0e20cfedb704c1f76bb727e2009867d3d503a03d Disconnect peers sending wtxidrelay message after VERACK (Suhas Daftuar) cacd85209e20fc0fd08f86eed23b6ef93484ffcf test: Use wtxid relay generally in functional tests (Fabian Jahr) 8d8099e97ab8af2126f6fbd223fbd82c52f2e85e test: Add tests for wtxid tx relay in segwit test (Fabian Jahr) 9a5392fdf67f1c5c90f52d3cdb3dea4f35d1609f test: Update test framework p2p protocol version to 70016 (Fabian Jahr) dd78d1d641178b473ab1156b71a837b9e686792b Rename AddInventoryKnown() to AddKnownTx() (Suhas Daftuar) 4eb515574e1012bc8ea5dafc3042dcdf4c766f26 Make TX_WITNESS_STRIPPED its own rejection reason (Suhas Daftuar) 97141ca442daea8fc9c307cf81a02b38dcc28fd8 Delay getdata requests from peers using txid-based relay (Suhas Daftuar) 46d78d47dea345329ba094310eec56ab00a02ddc Add p2p message "wtxidrelay" (Suhas Daftuar) 2d282e0cba9761574b6b43d134ca95f3052d7fd2 ignore non-wtxidrelay compliant invs (Anthony Towns) ac88e2eb619821ad7ae1d45d4b40be69051d3999 Add support for tx-relay via wtxid (Suhas Daftuar) 8e68fc246d09f1e6c6dfa8c676969d97c2eb4334 Add wtxids to recentRejects instead of txids (Suhas Daftuar) 144c38582042c3b9ec53bb97ba0644fc0114f8fb Add wtxids of confirmed transactions to bloom filter (Suhas Daftuar) 85c78d54af462996a0bca6cf97f91e1aa8778ae8 Add wtxid-index to orphan map (Suhas Daftuar) 08b39955ec7f84e835ab0b1366f0dd28dfd6ce03 Add a wtxid-index to mapRelay (Suhas Daftuar) 60f0acda713e7b9dc188aef54ef93981a93f4e44 Just pass a hash to AddInventoryKnown (Suhas Daftuar) c7eb6b4f1fe5bd76388a023529977674534334a7 Add wtxid to mempool unbroadcast tracking (Amiti Uttarwar) 2b4b90aa8f0440deacefb5997d7bd1f9f5c591b3 Add a wtxid-index to the mempool (Suhas Daftuar) Pull request description: Using txids (a transaction's hash, without witness) for transaction relay is problematic, post-segwit -- if a peer gives us a segwit transaction that fails policy checks, it could be because the txid associated with the transaction is definitely unacceptable to our node (regardless of the witness), or it could be that the transaction was malleated and with a different witness, the txid could be accepted to our mempool. We have a bloom filter of recently rejected transactions, whose purpose is to help us avoid redownloading and revalidating transactions that fail to be accepted, but because of this potential for witness malleability to interfere with relay of valid transactions, we do not use the filter for segwit transactions. This issue is discussed at some length in #8279. The effect of this is that whenever a segwit transaction that fails policy checks is relayed, a node would download that transaction from every peer announcing it, because it has no way presently to cache failure. Historically this hasn't been a big problem, but if/when policy for accepting segwit transactions were to change (eg taproot, or any other change), we could expect older nodes talking to newer nodes to be wasting bandwidth because of this. As discussed in that issue, switching to wtxid-based relay solves this problem -- by using an identifier for a transaction that commits to all the data in our relay protocol, we can be certain if a transaction that a peer is announcing is one that we've already tried to process, or if it's something new. This PR introduces support for wtxid-based relay with peers that support it (and remains backwards compatible with peers that use txids for relay, of course). Apart from code correctness, one issue to be aware of is that by downloading from old and new peers alike, we should expect there to be some bandwidth wasted, because sometimes we might download the same transaction via txid-relay as well as wtxid-relay. The last commit in this PR implements a heuristic I want to analyze, which is to just delay relay from txid-relay peers by 2 seconds, if we have at least 1 wtxid-based peer. I've just started running a couple nodes with this heuristic so I can measure how well it works, but I'm open to other ideas for minimizing that issue. In the long run, I think this will be essentially a non-issue, so I don't think it's too big a concern, we just need to bite the bullet and deal with it during upgrade. Finally, this proposal would need a simple BIP describing the changes, which I haven't yet drafted. However, review and testing of this code in the interim would be welcome. To do items: - [x] Write BIP explaining the spec here (1 new p2p message for negotiating wtxid-based relay, along with a new INV type) - [ ] Measure and evaluate a heuristic for minimizing how often a node downloads the same transaction twice, when connected to old and new nodes. ACKs for top commit: naumenkogs: utACK 0a4f1422cd1c20e12a05d7ff1a2ef1d5e7c654bb laanwj: utACK 0a4f1422cd1c20e12a05d7ff1a2ef1d5e7c654bb Tree-SHA512: d8eb8f0688cf0cbe9507bf738e143edab1f595551fdfeddc2b6734686ea26e7f156b6bfde38bad8bbbe8bec1857c7223e1687f8f018de7463dde8ecaa8f450df
2020-07-19Make TX_WITNESS_STRIPPED its own rejection reasonSuhas Daftuar
Previously, TX_WITNESS_MUTATED could be returned during transaction validation for either transactions that had a witness that was non-standard, or for transactions that had no witness but were invalid due to segwit validation rules. However, for txid/wtxid-relay considerations, net_processing distinguishes the witness stripped case separately, because it affects whether a wtxid should be able to be added to the reject filter. It is safe to add the wtxid of a witness-mutated transaction to the filter (as that wtxid shouldn't collide with the txid, and hence it wouldn't interfere with transaction relay from txid-relay peers), but it is not safe to add the wtxid (== txid) of a witness-stripped transaction to the filter, because that would interfere with relay of another transaction with the same txid (but different wtxid) when relaying from txid-relay peers. Also updates the comment explaining this logic, and explaining that we can get rid of this complexity once there's a sufficient deployment of wtxid-relaying peers on the network.
2020-07-15refactor: Switch ValidationState mode to C++11 enum classMarcoFalke
generated by cgit v1.2.3 (git 2.26.2) at 2024-12-13 08:37:13 +0000