* llmq: Avoid writing commitments to evodb and altering caches when all we want is to check block candidate validity
* tests: call `getblocktemplate` to trigger `CreateNewBlock` before quorum commitment is mined
92f1f8b31 Split off key_io_tests from base58_tests (Pieter Wuille)
119b0f85e Split key_io (address/key encodings) off from base58 (Pieter Wuille)
ebfe217b1 Stop using CBase58Data for ext keys (Pieter Wuille)
32e69fa0d Replace CBitcoinSecret with {Encode,Decode}Secret (Pieter Wuille)
Pull request description:
This PR contains some of the changes left as TODO in #11167 (and built on top of that PR). They are not intended for backporting.
This removes the `CBase58`, `CBitcoinSecret`, `CBitcoinExtKey`, and `CBitcoinExtPubKey` classes, in favor of simple `Encode`/`Decode` functions. Furthermore, all Bitcoin-specific logic (addresses, WIF, BIP32) is moved to `key_io.{h,cpp}`, leaving `base58.{h,cpp}` as a pure utility that implements the base58 encoding/decoding logic.
Tree-SHA512: a5962c0ed27ad53cbe00f22af432cf11aa530e3efc9798e25c004bc9ed1b5673db5df3956e398ee2c085e3a136ac8da69fe7a7d97a05fb2eb3be0b60d0479655
Make linter happy
Dashify
499d95e27 Add static_assert to prevent VARINT(<signed value>) (Russell Yanofsky)
Pull request description:
Using VARINT with signed types is dangerous because negative values will appear to serialize correctly, but then deserialize as positive values mod 128.
This commit changes the VARINT macro to trigger a compile error by default if called with an signed value, and it updates existing broken uses of VARINT to pass a special flag that lets them keep working with no changes in behavior.
There is some discussion about this issue here: https://github.com/bitcoin/bitcoin/pull/9693#issuecomment-278701473. I think another good change along these lines would be to make `GetSizeOfVarInt` and `WriteVarInt` throw exceptions if they are passed numbers less than 0 to serialize. But unlike this change, that would be a change in runtime behavior, and need more consideration.
Tree-SHA512: 082c65598cfac6dc1da042bdb47dbc9d5d789fc849fe52921cc238578588f4e5ff976c8b4b2ce42cb75290eb14f3b42ea76e26202c223c5b2aa63ef45c2ea3cc
* Fix potential deadlock in `CSporkManager::UpdateSpork()`
* Protect `inputRequestIds` with cs lock
* Protect `curDBTransaction` in `CEvoDB::CommitRootTransaction()`
* Check for `AssertLockNotHeld` in `EnforceBestChainLock()` instead of just having a comment in code
* Protect spork maps on (de)serialization
* Handle situations when we have evodb records but no blocks anymore
* Use IsEmpty
* Apply suggestions from code review
Co-authored-by: dustinface <35775977+xdustinface@users.noreply.github.com>
Co-authored-by: dustinface <35775977+xdustinface@users.noreply.github.com>
* Adjust CDeterministicMNState, add helper methods
Changes all setting of `nPoSeBanHeight` into a call to `BanIfNotBanned`
Adds a helper method `IsBanned` that just is `return nPoSeBanHeight != -1`
In my opinion this makes the code generally more readable and easy to understand
Adds `Revive` helper method
I don't feel too strongly about this, because from what I have seen, this revive code is only done in one place,
but I generally think it makes sense to be a helper method of it's own
Signed-off-by: pasta <pasta@dashboost.org>
* Add `!` that was accidentally not added
Signed-off-by: pasta <pasta@dashboost.org>
* Make nPoSeBanHeight private
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
* Make internalId private to make sure it's set/accessed properly
* evo: Make internalId only accessible by the constructor
* Tweak constructors
Co-authored-by: xdustinface <xdustinfacex@gmail.com>
* Return invalid state for GetTxPayload failures instead of asserting
* Throw an error when internal `find` fails instead of asserting
* Unify mnlist checks for lists created from blocks and lists created via updates of older lists with diffs
* Wrap evo block processing in try..catch to make sure all earlier introduced throws are caught
* evo/llmq: Adjust reject reasons for CFinalCommitmentTxPayload
In CheckLLMQCommitment this reject string was used so i adjusted the
other one in CQuorumBlockProcessor::GetCommitmentsFromBlock and the new
introduced in PR #3506
* return state.DoS instead of asserting in cbtx.cpp
Signed-off-by: pasta <pasta@dashboost.org>
Co-authored-by: xdustinface <xdustinfacex@gmail.com>
Co-authored-by: pasta <pasta@dashboost.org>
Deleting these from the DB means that future calls to GetListForBlock for
the reorged/orphaned block will return an empty MN list. At the same time,
GetListForBlock will add the empty list to the internal MN list cache.
This behavior is not acceptable for multiple reasons:
1. A caller of GetListForBlock should always expect a valid list if that
block was once in the best chain. We might otherwise run into strange
behavior (e.g. GetAllQuorumMembers returning zero members)
2. If the chain is ever reorged back to the initial chain (that was
orphaned), GetListForBlock should not return the invalid/empty cache
entry for the previously orphaned block.
This should also fix test failures seen on Gitlab CI
* scripted-diff: Replace #include "" with #include <> (ryanofsky)
-BEGIN VERIFY SCRIPT-
for f in \
src/*.cpp \
src/*.h \
src/bench/*.cpp \
src/bench/*.h \
src/compat/*.cpp \
src/compat/*.h \
src/consensus/*.cpp \
src/consensus/*.h \
src/crypto/*.cpp \
src/crypto/*.h \
src/crypto/ctaes/*.h \
src/policy/*.cpp \
src/policy/*.h \
src/primitives/*.cpp \
src/primitives/*.h \
src/qt/*.cpp \
src/qt/*.h \
src/qt/test/*.cpp \
src/qt/test/*.h \
src/rpc/*.cpp \
src/rpc/*.h \
src/script/*.cpp \
src/script/*.h \
src/support/*.cpp \
src/support/*.h \
src/support/allocators/*.h \
src/test/*.cpp \
src/test/*.h \
src/wallet/*.cpp \
src/wallet/*.h \
src/wallet/test/*.cpp \
src/wallet/test/*.h \
src/zmq/*.cpp \
src/zmq/*.h
do
base=${f%/*}/ relbase=${base#src/} sed -i "s:#include \"\(.*\)\"\(.*\):if test -e \$base'\\1'; then echo \"#include <\"\$relbase\"\\1>\\2\"; else echo \"#include <\\1>\\2\"; fi:e" $f
done
-END VERIFY SCRIPT-
Signed-off-by: Pasta <pasta@dashboost.org>
* scripted-diff: Replace #include "" with #include <> (Dash Specific)
-BEGIN VERIFY SCRIPT-
for f in \
src/bls/*.cpp \
src/bls/*.h \
src/evo/*.cpp \
src/evo/*.h \
src/governance/*.cpp \
src/governance/*.h \
src/llmq/*.cpp \
src/llmq/*.h \
src/masternode/*.cpp \
src/masternode/*.h \
src/privatesend/*.cpp \
src/privatesend/*.h
do
base=${f%/*}/ relbase=${base#src/} sed -i "s:#include \"\(.*\)\"\(.*\):if test -e \$base'\\1'; then echo \"#include <\"\$relbase\"\\1>\\2\"; else echo \"#include <\\1>\\2\"; fi:e" $f
done
-END VERIFY SCRIPT-
Signed-off-by: Pasta <pasta@dashboost.org>
* build: Remove -I for everything but project root
Remove -I from build system for everything but the project root,
and built-in dependencies.
Signed-off-by: Pasta <pasta@dashboost.org>
# Conflicts:
# src/Makefile.test.include
* qt: refactor: Use absolute include paths in .ui files
* qt: refactor: Changes to make include paths absolute
This makes all include paths in the GUI absolute.
Many changes are involved as every single source file in
src/qt/ assumes to be able to use relative includes.
Signed-off-by: Pasta <pasta@dashboost.org>
# Conflicts:
# src/qt/dash.cpp
# src/qt/optionsmodel.cpp
# src/qt/test/rpcnestedtests.cpp
* test: refactor: Use absolute include paths for test data files
* Recommend #include<> syntax in developer notes
* refactor: Include obj/build.h instead of build.h
* END BACKPORT #11651 Remove trailing whitespace causing travis failure
* fix backport 11651
Signed-off-by: Pasta <pasta@dashboost.org>
* More of 11651
* fix blockchain.cpp
Signed-off-by: pasta <pasta@dashboost.org>
* Add missing "qt/" in includes
* Add missing "test/" in includes
* Fix trailing whitespaces
Co-authored-by: Wladimir J. van der Laan <laanwj@gmail.com>
Co-authored-by: Russell Yanofsky <russ@yanofsky.org>
Co-authored-by: MeshCollider <dobsonsa68@gmail.com>
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
* Masternodes must have required services enabled
* Add a comment about missing bits
* Refactor *InitParameterInteraction parts to follow surrounding code logic
Changes:
- move all related param interactions to appropriate places;
- try to softly set required params, complain and fail to start later if smth is not ok.
* Drop redundant code
a357293 Use MakeUnique<Db>(...) (practicalswift)
3e09b39 Use MakeUnique<T>(...) instead of std::unique_ptr<T>(new T(...)) (practicalswift)
8617989 Add MakeUnique (substitute for C++14 std::make_unique) (practicalswift)
d223bc9 Use unique_ptr for pcoinscatcher/pcoinsdbview/pcoinsTip/pblocktree (practicalswift)
b45c597 Use unique_ptr for pdbCopy (Db) and fix potential memory leak (practicalswift)
29ab96d Use unique_ptr for dbenv (DbEnv) (practicalswift)
f72cbf9 Use unique_ptr for pfilter (CBloomFilter) (practicalswift)
8ccf1bb Use unique_ptr for sem{Addnode,Outbound} (CSemaphore) (practicalswift)
73db063 Use unique_ptr for upnp_thread (boost::thread) (practicalswift)
0024531 Use unique_ptr for dbw (CDBWrapper) (practicalswift)
fa6d122 Use unique_ptr:s for {fee,short,long}Stats (TxConfirmStats) (practicalswift)
5a6f768 Use unique_ptr for httpRPCTimerInterface (HTTPRPCTimerInterface) (practicalswift)
860e912 Use unique_ptr for pwalletMain (CWallet) (practicalswift)
Pull request description:
Use `std::unique_ptr` (C++11) where possible.
Rationale:
1. Avoid resource leaks (specifically: forgetting to `delete` an object created using `new`)
2. Avoid undefined behaviour (specifically: double `delete`:s)
**Note to reviewers:** Please let me know if I've missed any obvious `std::unique_ptr` candidates. Hopefully this PR should cover all the trivial cases.
Tree-SHA512: 9fbeb47b800ab8ff4e0be9f2a22ab63c23d5c613a0c6716d9183db8d22ddbbce592fb8384a8b7874bf7375c8161efb13ca2197ad6f24b75967148037f0f7b20c
* Merge #11117: Prepare for non-Base58 addresses
864cd2787 Move CBitcoinAddress to base58.cpp (Pieter Wuille)
5c8ff0d44 Introduce wrappers around CBitcoinAddress (Pieter Wuille)
Pull request description:
This patch removes the need for the intermediary Base58 type `CBitcoinAddress`, by providing {`Encode`,`Decode`,`IsValid`}`Destination` functions that directly operate on the conversion between `std::string`s and `CTxDestination`.
As a side, it also fixes a number of indentation issues, and removes probably several unnecessary implicit `CTxDestination`<->`CBitcoinAddress` conversions.
This change is far from complete. In follow-ups I'd like to:
* Split off the specific address and key encoding logic from base58.h, and move it to a address.h or so.
* Replace `CTxDestination` with a non-`boost::variant` version (which can be more efficient as `boost::variant` allocates everything on the heap, and remove the need for `boost::get<...>` and `IsValidDestination` calls everywhere).
* Do the same for `CBitcoinSecret`, `CBitcoinExtKey`, and `CBitcoinExtPubKey`.
However, I've tried to keep this patch to be minimally invasive, but still enough to support non-Base58 addresses. Perhaps a smaller patch is possible to hack Bech32 support into `CBitcoinAddress`, but I would consider that a move in the wrong direction.
Tree-SHA512: c2c77ffb57caeadf2429b1c2562ce60e8c7be8aa9f8e51b591f354b6b441162625b2efe14c023a1ae485cf2ed417263afa35c892891dfaa7844e7fbabccab85e
* CBitcoinAddress -> EncodeDestination in providertx.h
Signed-off-by: Pasta <pasta@dashboost.org>
* more CBitcoinAddress -> EncodeDestination in providertx.h
Signed-off-by: Pasta <pasta@dashboost.org>
* more CBitcoinAddress -> EncodeDestination in providertx.h
Signed-off-by: Pasta <pasta@dashboost.org>
* more CBitcoinAddress -> EncodeDestination in providertx.h
Signed-off-by: Pasta <pasta@dashboost.org>
* fix CBitcoinAddress GetKeyID check
Signed-off-by: Pasta <pasta@dashboost.org>
* fix providertx.cpp
Signed-off-by: Pasta <pasta@dashboost.org>
* hopefully fix governance-classes.cpp
Signed-off-by: Pasta <pasta@dashboost.org>
* partially fix governance-validators.cpp, unable to resolve "address.IsScript()"
Signed-off-by: Pasta <pasta@dashboost.org>
* partially fix governance-classes.cpp, unable to resolve "address.IsScript()"
Signed-off-by: Pasta <pasta@dashboost.org>
* fix governance-classes.h
Signed-off-by: Pasta <pasta@dashboost.org>
* DecodeTransaction -> DecodeDestination, fix governance-validators.cpp
Signed-off-by: Pasta <pasta@dashboost.org>
* More fixes for 3294
* Move GetIndexKey into rpc/misc.cpp near getAddressesFromParams
No need to have it in base58.cpp anymore as this is only used in getAddressesFromParams
Co-authored-by: Wladimir J. van der Laan <laanwj@gmail.com>
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
Co-authored-by: Alexander Block <ablock84@gmail.com>
This has the wanted side effect of proper locking of "cs" inside
CommitCurTransaction and RollbackCurTransaction, which was not easily
possible to implement in the generic version. This fixes some rare crashes.
