Age | Commit message (Collapse) | Author |
|
8d491ae9ecf1948ea29f67b50ca7259123f602aa serialization: Add ParamsStream GetStream() method (Ryan Ofsky)
951203bcc496c4415b7754cd764544659b76067f net: Simplify ParamsStream usage (Ryan Ofsky)
e6794e475c84d9edca4a2876e2342cbb1d85f804 serialization: Accept multiple parameters in ParamsStream constructor (Ryan Ofsky)
cb28849a88339c1e7ba03ffc7e38998339074e6e serialization: Reverse ParamsStream constructor order (Ryan Ofsky)
83436d14f06551026bcf5529df3b63b4e8a679fb serialization: Drop unnecessary ParamsStream references (Ryan Ofsky)
84502b755bcc35413ad466047893b5edf134c53f serialization: Drop references to GetVersion/GetType (Ryan Ofsky)
f3a2b5237688e9f574444e793724664b00fb7f2a serialization: Support for multiple parameters (Ryan Ofsky)
Pull request description:
Currently it is only possible to attach one serialization parameter to a stream at a time. For example, it is not possible to set a parameter controlling the transaction format and a parameter controlling the address format at the same time because one parameter will override the other.
This limitation is inconvenient for multiprocess code since it is not possible to create just one type of stream and serialize any object to it. Instead it is necessary to create different streams for different object types, which requires extra boilerplate and makes using the new parameter fields a lot more awkward than the older version and type fields.
Fix this problem by allowing an unlimited number of serialization stream parameters to be set, and allowing them to be requested by type. Later parameters will still override earlier parameters, but only if they have the same type.
For an example of different ways multiple parameters can be set, see the new [`with_params_multi`](https://github.com/bitcoin/bitcoin/blob/40f505583f4edeb2859aeb70da20c6374d331a4f/src/test/serialize_tests.cpp#L394-L410) unit test.
This change requires replacing the `stream.GetParams()` method with a `stream.GetParams<T>()` method in order for serialization code to retrieve the desired parameters. The change is more verbose, but probably a good thing for readability because previously it could be difficult to know what type the `GetParams()` method would return, and now it is more obvious.
---
This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/issues/28722).
ACKs for top commit:
maflcko:
ACK 8d491ae9ecf1948ea29f67b50ca7259123f602aa 🔵
sipa:
utACK 8d491ae9ecf1948ea29f67b50ca7259123f602aa
TheCharlatan:
ACK 8d491ae9ecf1948ea29f67b50ca7259123f602aa
Tree-SHA512: 40b7041ee01c0372b1f86f7fd6f3b6af56ef24a6383f91ffcedd04d388e63407006457bf7ed056b0e37b4dec9ffd5ca006cb8192e488ea2c64678567e38d4647
|
|
Slight performance improvement by avoiding duplicate work.
|
|
Drop unnecessary ParamsStream references from CTransaction and
CMutableTransaction constructors. This just couples these classes unnecessarily
to the ParamsStream class, making the ParamsStream class harder to modify, and
making the transaction classes in some cases (depending on parameter order)
unable to work with stream classes that have multiple parameters set.
|
|
This commit makes a minimal change to the ParamsStream class to let it retrieve
multiple parameters. Followup commits after this commit clean up code using
ParamsStream and make it easier to set multiple parameters.
Currently it is only possible to attach one serialization parameter to a stream
at a time. For example, it is not possible to set a parameter controlling the
transaction format and a parameter controlling the address format at the same
time because one parameter will override the other.
This limitation is inconvenient for multiprocess code since it is not possible
to create just one type of stream and serialize any object to it. Instead it is
necessary to create different streams for different object types, which
requires extra boilerplate and makes using the new parameter fields a lot more
awkward than the older version and type fields.
Fix this problem by allowing an unlimited number of serialization stream
parameters to be set, and allowing them to be requested by type. Later
parameters will still override earlier parameters, but only if they have the
same type.
This change requires replacing the stream.GetParams() method with a
stream.GetParams<T>() method in order for serialization code to retrieve the
desired parameters. This change is more verbose, but probably a good thing for
readability because previously it could be difficult to know what type the
GetParams() method would return, and now it is more obvious.
|
|
(28107 follow-up)
af1d2ff88344e1545ac8d9ad09f8e37e264da712 [primitives] Precompute result of CTransaction::HasWitness (dergoegge)
Pull request description:
This addresses https://github.com/bitcoin/bitcoin/pull/28107#discussion_r1364961590 from #28107.
ACKs for top commit:
theStack:
ACK af1d2ff88344e1545ac8d9ad09f8e37e264da712
achow101:
ACK af1d2ff88344e1545ac8d9ad09f8e37e264da712
stickies-v:
ACK af1d2ff88344e1545ac8d9ad09f8e37e264da712
TheCharlatan:
ACK af1d2ff88344e1545ac8d9ad09f8e37e264da712
Tree-SHA512: a77654ae429d0d7ce12daa309770e75beec4f8984734f80ed203156199425af43b50ad3d8aab85a89371a71356464ebd4503a0248fd0103579adfc74a55aaf51
|
|
|
|
|
|
|
|
|
|
|
|
Then, use the compiler warnings to create copies only where needed.
Also, fix iwyu includes while touching the includes.
|
|
|
|
The type is only ever set, but never read via GetType(), so remove it.
Also, remove SerializeHash to avoid silent merge conflicts and use the
already existing GetHash() boilerplate consistently.
|
|
|
|
Co-authored-by: Pieter Wuille <pieter@wuille.net>
|
|
-BEGIN VERIFY SCRIPT-
./contrib/devtools/copyright_header.py update ./
-END VERIFY SCRIPT-
Commits of previous years:
- 2021: f47dda2c58b5d8d623e0e7ff4e74bc352dfa83d7
- 2020: fa0074e2d82928016a43ca408717154a1c70a4db
- 2019: aaaaad6ac95b402fe18d019d67897ced6b316ee0
|
|
It doesn't make sense to be checking whether the fee paid is underpaying
before we've finished setting the fees. So do that after we have done
the reduction for SFFO and change adjustment for fee overpayment.
|
|
Co-authored-by: "Pieter Wuille <pieter@wuille.net>"
Co-authored-by: "Vasil Dimov <vd@FreeBSD.org>"
Co-authored-by: "MarcoFalke <falke.marco@gmail.com>"
|
|
To be used in the next commit
|
|
It involves calculating two hashes, so the performance impact should be
made explicit.
Also, add the module to iwyu.
|
|
deserialization docstring
d4b3483cece9c27d58e4026df35725655ce06cf5 Primitives: Correct CTransaction deserialization docstring (TheCharlatan)
Pull request description:
Since https://github.com/bitcoin/bitcoin/pull/8589 CTxWitness was removed and instead replaced with CScriptWitness inside each CTxIn.
ACKs for top commit:
w0xlt:
ACK d4b3483
Tree-SHA512: 02bb73e8a7d1fc449e4776a162009261baecc573837fade74ad7d76b3cd63200424e02fd0abd000c63706072f2ab3c95d3053139495b81347463f43e56192ca9
|
|
Since https://github.com/bitcoin/bitcoin/pull/8589 CTxWitness was
removed and instead replaced with CScriptWitness inside each CTxIn.
|
|
|
|
-BEGIN VERIFY SCRIPT-
./contrib/devtools/copyright_header.py update ./
-END VERIFY SCRIPT-
Commits of previous years:
* 2020: fa0074e2d82928016a43ca408717154a1c70a4db
* 2019: aaaaad6ac95b402fe18d019d67897ced6b316ee0
|
|
|
|
|
|
Move amount.h to consensus/amount.h.
Renames, adds missing and removes uneeded includes.
|
|
-BEGIN VERIFY SCRIPT-
./contrib/devtools/copyright_header.py update ./
-END VERIFY SCRIPT-
|
|
fade6195b1c230edd561443637a7bde81c2594a4 Move TX_MAX_STANDARD_VERSION to policy (MarcoFalke)
Pull request description:
`primitives` should only be used for the raw datastructures (parsing and format). It is not the right place to document relay policy.
ACKs for top commit:
laanwj:
Code review ACK fade6195b1c230edd561443637a7bde81c2594a4
lontivero:
Concept ACK https://github.com/bitcoin/bitcoin/pull/20611/commits/fade6195b1c230edd561443637a7bde81c2594a4
Tree-SHA512: f809c4aecd14d7e9feaa7b50b9c0697232991eef36190cd960bcfb0ad6e20c71a4f6aab48c7747cf8a681eb14feda60c55b09a37f128673d519567224f29cd97
|
|
Also remove extraneous whitespace, should be reviewed with --ignore-all-space
|
|
|
|
|
|
Co-authored-by: Carl Dong <contact@carldong.me>
|
|
|
|
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
|
|
|
|
-BEGIN VERIFY SCRIPT-
./contrib/devtools/copyright_header.py update ./
-END VERIFY SCRIPT-
|
|
|
|
|
|
-BEGIN VERIFY SCRIPT-
./contrib/devtools/copyright_header.py update ./
-END VERIFY SCRIPT-
|
|
|
|
|
|
bb530efa18 Disallow extended encoding for non-witness transactions (Pieter Wuille)
Pull request description:
BIP144 specifies that transactions without witness should use the legacy encoding, which is currently not enforced.
This rule was present in the original SegWit implementation (https://github.com/bitcoin/bitcoin/pull/8149), but was subsequently dropped (https://github.com/bitcoin/bitcoin/pull/8589).
As all hashes, txids, and weights are always computed over a reserialized version of a transaction, it is mostly harmless to permit extended encoding for non-segwit transactions, but I'd rather strictly follow the BIP.
ACKs for commit bb530e:
instagibbs:
utACK https://github.com/bitcoin/bitcoin/pull/14039/commits/bb530efa1872ec963417f61da9a95185c7a7a7d6
stevenroose:
utACK bb530efa1872ec963417f61da9a95185c7a7a7d6
Tree-SHA512: 1aeccd6a555f43784fefb076ce2e8ad2f5ba7be49840544a50050d0390f82373f87201bf56cf8bb30841b4f9cd893b382261a080da875d4e11ab7051f8640dbe
|
|
This makes the above constructor explicit. The rationale is that this conversion has very significant performance effects. Making it explicit makes it easier to reason about these performance trade-offs, and helps identify possible functions that need a CMutableTransaction version.
|
|
|
|
-BEGIN VERIFY SCRIPT-
mkdir -p src/util
git mv src/util.h src/util/system.h
git mv src/util.cpp src/util/system.cpp
git mv src/utilmemory.h src/util/memory.h
git mv src/utilmoneystr.h src/util/moneystr.h
git mv src/utilmoneystr.cpp src/util/moneystr.cpp
git mv src/utilstrencodings.h src/util/strencodings.h
git mv src/utilstrencodings.cpp src/util/strencodings.cpp
git mv src/utiltime.h src/util/time.h
git mv src/utiltime.cpp src/util/time.cpp
sed -i 's/<util\.h>/<util\/system\.h>/g' $(git ls-files 'src/*.h' 'src/*.cpp')
sed -i 's/<utilmemory\.h>/<util\/memory\.h>/g' $(git ls-files 'src/*.h' 'src/*.cpp')
sed -i 's/<utilmoneystr\.h>/<util\/moneystr\.h>/g' $(git ls-files 'src/*.h' 'src/*.cpp')
sed -i 's/<utilstrencodings\.h>/<util\/strencodings\.h>/g' $(git ls-files 'src/*.h' 'src/*.cpp')
sed -i 's/<utiltime\.h>/<util\/time\.h>/g' $(git ls-files 'src/*.h' 'src/*.cpp')
sed -i 's/BITCOIN_UTIL_H/BITCOIN_UTIL_SYSTEM_H/g' src/util/system.h
sed -i 's/BITCOIN_UTILMEMORY_H/BITCOIN_UTIL_MEMORY_H/g' src/util/memory.h
sed -i 's/BITCOIN_UTILMONEYSTR_H/BITCOIN_UTIL_MONEYSTR_H/g' src/util/moneystr.h
sed -i 's/BITCOIN_UTILSTRENCODINGS_H/BITCOIN_UTIL_STRENCODINGS_H/g' src/util/strencodings.h
sed -i 's/BITCOIN_UTILTIME_H/BITCOIN_UTIL_TIME_H/g' src/util/time.h
sed -i 's/ util\.\(h\|cpp\)/ util\/system\.\1/g' src/Makefile.am
sed -i 's/utilmemory\.\(h\|cpp\)/util\/memory\.\1/g' src/Makefile.am
sed -i 's/utilmoneystr\.\(h\|cpp\)/util\/moneystr\.\1/g' src/Makefile.am
sed -i 's/utilstrencodings\.\(h\|cpp\)/util\/strencodings\.\1/g' src/Makefile.am
sed -i 's/utiltime\.\(h\|cpp\)/util\/time\.\1/g' src/Makefile.am
sed -i 's/-> util ->/-> util\/system ->/' test/lint/lint-circular-dependencies.sh
sed -i 's/src\/util\.cpp/src\/util\/system\.cpp/g' test/lint/lint-format-strings.py test/lint/lint-locale-dependence.sh
sed -i 's/src\/utilmoneystr\.cpp/src\/util\/moneystr\.cpp/g' test/lint/lint-locale-dependence.sh
sed -i 's/src\/utilstrencodings\.\(h\|cpp\)/src\/util\/strencodings\.\1/g' test/lint/lint-locale-dependence.sh
sed -i 's/src\\utilstrencodings\.cpp/src\\util\\strencodings\.cpp/' build_msvc/libbitcoinconsensus/libbitcoinconsensus.vcxproj
-END VERIFY SCRIPT-
|
|
|
|
|
|
|
|
|