Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Properly handle conflicts between ChainLocks and InstantSend #2904

Merged
merged 17 commits into from
May 7, 2019

Conversation

codablock
Copy link

@codablock codablock commented May 6, 2019

This PR adds code to handle conflicts between ChainLocks and InstantSend. The probability of such conflicts is practically zero (due to the "safe TXs" concept, as described here), but these should still be handled properly to assure consensus.

This PR lacks tests atm, which I'm working on now. We can either merge this PR first and then later merge the tests, or wait with merging until I'm done with the tests. Update: Added tests

@codablock codablock added this to the 14.0 milestone May 6, 2019
Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! 👍

Since this PR touches some consensus critical stuff I think it would make sense to wait and include tests here.

(Also, this can be rebased now to remove eb18e19)

src/validation.cpp Outdated Show resolved Hide resolved
@PastaPastaPasta
Copy link
Member

Agree with Udjin on waiting for tests

We'll need this from another method as well later.
These allow to easily delete multiple chains (actually trees) of ISLOCKs
in one go.
…orLock

Also add "retryChildren" parameter to RemoveNonLockedTx so that we can
skip retrying of non-locked children TXs.
But only when a block is known to have a conflict and at the same time is
ChainLocked, which causes the ISLOCK to be pruned.
Bail out (continue) early
@codablock
Copy link
Author

Added tests and appropriate fixes to make everything work as expected. Also rebased on develop.

Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK 👍

@@ -968,7 +968,7 @@ void CInstantSendManager::SyncTransaction(const CTransaction& tx, const CBlockIn
if (!chainlocked && islockHash.IsNull()) {
// TX is not locked, so make sure it is tracked
AddNonLockedTx(MakeTransactionRef(tx));
nonLockedTxs.at(tx.GetHash()).pindexMined = posInBlock == CMainSignals::SYNC_TRANSACTION_NOT_IN_BLOCK ? pindex : nullptr;
nonLockedTxs.at(tx.GetHash()).pindexMined = posInBlock != CMainSignals::SYNC_TRANSACTION_NOT_IN_BLOCK ? pindex : nullptr;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, good catch!

@UdjinM6 UdjinM6 merged commit 66a2cde into dashpay:develop May 7, 2019
@UdjinM6 UdjinM6 added the RPC Some notable changes to RPC params/behaviour/descriptions label May 7, 2019
@codablock codablock deleted the pr_is_invalidateblocks branch May 8, 2019 09:57
MIPPL pushed a commit to biblepay/biblepay that referenced this pull request Jun 16, 2019
* commit '05adda99fe09f9f6d99ce09e22ed89be3ddfcd27': (530 commits)
  Update release notes with latest commits (dashpay#2958)
  Only require valid collaterals for votes and triggers (dashpay#2947) (dashpay#2957)
  [v0.14.0.x] Fix off-by-one error in InstantSend mining info removal when disconnecting blocks (dashpay#2951)
  [v0.14.0.x] bump version to 0.14.0.1 and prepare release notes (dashpay#2952)
  Update release notes v14 (dashpay#2927)
  Set CLIENT_VERSION_IS_RELEASE to true (dashpay#2926)
  Update help text via gen-manpages.sh (dashpay#2929)
  0.14 release notes draft (dashpay#2896)
  Fix duplicate `-instantsendnotify` invocation (dashpay#2925)
  Add blocks conflicting with ChainLocks to block index (dashpay#2923)
  Skip processing in SyncTransaction when chain is not synced yet (dashpay#2920)
  Set DIP0008 mainnet activation params (dashpay#2915)
  [0.14] Bump chainparams (dashpay#2910)
  Fix db leaks in LLMQ db (dashpay#2914)
  Fall back to ReadBlockFromDisk when blockTxs is not filled yet (dashpay#2908)
  Bump "keepOldConnections" by one for all LLMQ types (dashpay#2909)
  Print inputs on which we voted and quorums used for signing (dashpay#2907)
  Implement integration tests for DKG error handling (dashpay#2905)
  Implement zmq notifications for chainlocked blocks (dashpay#2899)
  Properly handle conflicts between ChainLocks and InstantSend (dashpay#2904)
  ...

# Conflicts:
#	.travis.yml
#	COPYING
#	README.md
#	biblepay-docs/protocol-documentation.md
#	ci/matrix.sh
#	configure.ac
#	contrib/debian/examples/biblepay.conf
#	contrib/gitian-descriptors/gitian-linux.yml
#	contrib/gitian-descriptors/gitian-osx.yml
#	contrib/gitian-descriptors/gitian-win.yml
#	doc/README.md
#	doc/README_windows.txt
#	doc/dnsseed-policy.md
#	doc/files.md
#	doc/guide-startmany.md
#	doc/man/biblepay-cli.1
#	doc/man/biblepay-qt.1
#	doc/man/biblepay-tx.1
#	doc/man/biblepayd.1
#	doc/masternode-budget.md
#	doc/masternode_conf.md
#	doc/release-notes.md
#	qa/pull-tester/rpc-tests.py
#	qa/rpc-tests/autoix-mempool.py
#	qa/rpc-tests/dip3-deterministicmns.py
#	qa/rpc-tests/fundrawtransaction.py
#	qa/rpc-tests/maxblocksinflight.py
#	qa/rpc-tests/p2p-acceptblock.py
#	qa/rpc-tests/test_framework/comptool.py
#	qa/rpc-tests/test_framework/mininode.py
#	qa/rpc-tests/test_framework/test_framework.py
#	qa/rpc-tests/test_framework/util.py
#	qa/rpc-tests/wallet-hd.py
#	share/setup.nsi.in
#	src/Makefile.am
#	src/Makefile.bench.include
#	src/Makefile.qt.include
#	src/Makefile.qttest.include
#	src/Makefile.test.include
#	src/activemasternode.cpp
#	src/activemasternode.h
#	src/biblepayd.cpp
#	src/cachemap.h
#	src/cachemultimap.h
#	src/chainparams.cpp
#	src/clientversion.h
#	src/dsnotificationinterface.cpp
#	src/evo/deterministicmns.cpp
#	src/evo/providertx.cpp
#	src/evo/simplifiedmns.cpp
#	src/governance-classes.cpp
#	src/governance-classes.h
#	src/governance-object.cpp
#	src/governance-object.h
#	src/governance-vote.cpp
#	src/governance.cpp
#	src/governance.h
#	src/hash.h
#	src/hdchain.cpp
#	src/hdchain.h
#	src/init.cpp
#	src/instantx.cpp
#	src/instantx.h
#	src/keepass.cpp
#	src/keepass.h
#	src/llmq/quorums_blockprocessor.cpp
#	src/llmq/quorums_dummydkg.cpp
#	src/llmq/quorums_dummydkg.h
#	src/llmq/quorums_init.cpp
#	src/llmq/quorums_init.h
#	src/llmq/quorums_utils.cpp
#	src/llmq/quorums_utils.h
#	src/masternode-payments.cpp
#	src/masternode-payments.h
#	src/masternode-sync.cpp
#	src/masternode-sync.h
#	src/masternode.cpp
#	src/masternode.h
#	src/masternodeconfig.cpp
#	src/masternodeconfig.h
#	src/masternodeman.cpp
#	src/masternodeman.h
#	src/messagesigner.cpp
#	src/messagesigner.h
#	src/miner.cpp
#	src/net.cpp
#	src/net_processing.cpp
#	src/netfulfilledman.cpp
#	src/netfulfilledman.h
#	src/privatesend-client.cpp
#	src/privatesend-client.h
#	src/privatesend-server.cpp
#	src/privatesend-server.h
#	src/privatesend.cpp
#	src/privatesend.h
#	src/qt/askpassphrasedialog.cpp
#	src/qt/biblepay.cpp
#	src/qt/bitcoinaddressvalidator.cpp
#	src/qt/bitcoingui.cpp
#	src/qt/bitcoinunits.cpp
#	src/qt/bitcoinunits.h
#	src/qt/clientmodel.cpp
#	src/qt/coincontroldialog.cpp
#	src/qt/dashstrings.cpp
#	src/qt/editaddressdialog.cpp
#	src/qt/forms/intro.ui
#	src/qt/forms/masternodelist.ui
#	src/qt/guiconstants.h
#	src/qt/guiutil.cpp
#	src/qt/intro.cpp
#	src/qt/locale/biblepay_ar.ts
#	src/qt/locale/biblepay_bg.ts
#	src/qt/locale/biblepay_de.ts
#	src/qt/locale/biblepay_en.ts
#	src/qt/locale/biblepay_es.ts
#	src/qt/locale/biblepay_fi.ts
#	src/qt/locale/biblepay_fr.ts
#	src/qt/locale/biblepay_it.ts
#	src/qt/locale/biblepay_ja.ts
#	src/qt/locale/biblepay_ko.ts
#	src/qt/locale/biblepay_nl.ts
#	src/qt/locale/biblepay_pl.ts
#	src/qt/locale/biblepay_pt.ts
#	src/qt/locale/biblepay_ru.ts
#	src/qt/locale/biblepay_sk.ts
#	src/qt/locale/biblepay_th.ts
#	src/qt/locale/biblepay_tr.ts
#	src/qt/locale/biblepay_vi.ts
#	src/qt/locale/biblepay_zh_CN.ts
#	src/qt/locale/biblepay_zh_TW.ts
#	src/qt/networkstyle.h
#	src/qt/openuridialog.cpp
#	src/qt/optionsdialog.cpp
#	src/qt/optionsmodel.cpp
#	src/qt/overviewpage.cpp
#	src/qt/res/images/light/about.png
#	src/qt/res/images/light/drkblue_walletFrame_bg.png
#	src/qt/rpcconsole.cpp
#	src/qt/sendcoinsdialog.cpp
#	src/qt/sendcoinsentry.cpp
#	src/qt/splashscreen.cpp
#	src/qt/transactiondesc.cpp
#	src/qt/transactionrecord.cpp
#	src/qt/utilitydialog.cpp
#	src/qt/walletmodel.cpp
#	src/rpc/blockchain.cpp
#	src/rpc/client.cpp
#	src/rpc/governance.cpp
#	src/rpc/masternode.cpp
#	src/rpc/mining.cpp
#	src/rpc/misc.cpp
#	src/rpc/net.cpp
#	src/rpc/rawtransaction.cpp
#	src/rpc/rpcevo.cpp
#	src/rpc/server.cpp
#	src/spork.cpp
#	src/spork.h
#	src/test/bip39_tests.cpp
#	src/test/cachemap_tests.cpp
#	src/test/cachemultimap_tests.cpp
#	src/test/governance_validators_tests.cpp
#	src/test/miner_tests.cpp
#	src/test/ratecheck_tests.cpp
#	src/test/test_biblepay.h
#	src/uint256.h
#	src/util.cpp
#	src/util.h
#	src/validation.cpp
#	src/validation.h
#	src/version.h
#	src/wallet/rpcdump.cpp
#	src/wallet/rpcwallet.cpp
#	src/wallet/wallet.cpp
#	src/wallet/wallet.h
#	src/wallet/walletdb.cpp
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RPC Some notable changes to RPC params/behaviour/descriptions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants