instantsend: Implement Spork 2 Mempool Signing signalling #4024

PastaPastaPasta · 2021-03-02T23:20:40Z

This PR implements a new Spork, SPORK_24_INSTANTSEND_SIGNING_ENABLED

Previously, the Dash network has seen InstantSend fail under extremely high load. These IS failures have resulted in diminished or non-operational chainlocks. We have worked to heavily optimize IS creation and propagation with the implementation of Concentrated Recovery as well as just pure optimization, however even with that, so much CPU can only do so much. It is likely that in order to massively scale InstantSend masternodes will need to upgrade their hardware. However, with current average usage, it makes almost no sense for masternodes to upgrade, as current hardware easily handles average load.

To mitigate these InstantSend failures further, there are a number of improvements we should investigate.

Implementation of Dynamic Fees based on mempool usage
Implementation of Dynamic Fees based on transaction throughput
Implementation of Dynamic Fees based on average InstantSend Lock creation time

There are also some future optimizations to look into (I haven't thought TOO much about these options so 🤷 )
4. Implementation of Input Lock batch creation & / or InstantSend Lock batch creation
5. Implementation of Input Lock batch propagation

I propose that even in the case that IS becomes overloaded, potentially in a sustained attack lasting a week or more, ChainLocks should remain operational. In order to achieve this, I propose that InstantSend be disabled in the case of InstantSend failures resulting in diminished or non-operational ChainLocks.

We could achieve this currently by simply disabling Spork 2, however I think that this has a couple of problems. After we disable spork 2, transactions that had previously been locked will no longer retain the protections of instantsend. Additionally, normal miners will no longer only include "safe" transactions. By introducing this Spork24, we can first disable spork24, at this point MNs will stop creating new input locks or instantsend locks for transactions only in mempool (however we keep creating islocks for transactions included in a block such that that block can be CLed ASAP). Once no transactions on the network are ISlocked, then Spork 2 can safely be disabled. At this point, miners can include any valid transactions (not just "safe" txes), and those blocks will be CLed.

LMK if you think of a better way to achieve this, it may be possible to just modify the behavior of Spork2, but I think this approach may be safer.

EDIT: Changed to using spork2 value 1 instead of spork 24 as indicating no mempool signing

Signed-off-by: pasta <pasta@dashboost.org>

This spork tells masternodes to refuse to lock transactions in mempool. Only transactions included in a block should be retroactively signed. Signed-off-by: pasta <pasta@dashboost.org> add spork defenition Signed-off-by: pasta <pasta@dashboost.org>

…lmq/*` Signed-off-by: pasta <pasta@dashboost.org>

Signed-off-by: pasta <pasta@dashboost.org>

… usage Signed-off-by: pasta <pasta@dashboost.org>

UdjinM6

Interesting idea 👍

src/llmq/quorums_instantsend.cpp

test/functional/feature_llmq_is_retroactive.py

src/llmq/quorums_instantsend.cpp

Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>

Signed-off-by: pasta <pasta@dashboost.org>

…a/dash into instantsend-signing-spork

PastaPastaPasta · 2021-03-04T00:29:12Z

See latest

Signed-off-by: pasta <pasta@dashboost.org>

xdustinface

See comments below + PR descriptions needs some update (spork24 removal) now.

test/functional/feature_llmq_is_retroactive.py

test/functional/test_framework/test_framework.py

test/functional/feature_llmq_is_retroactive.py

src/llmq/quorums_instantsend.cpp

xdustinface · 2021-03-05T13:08:55Z

src/llmq/quorums_instantsend.cpp

@@ -422,7 +439,7 @@ bool CInstantSendManager::ProcessTx(const CTransaction& tx, bool allowReSigning,
        if (quorumSigningManager->GetVoteForId(llmqType, id, otherTxHash)) {
            if (otherTxHash != tx.GetHash()) {
                LogPrintf("CInstantSendManager::%s -- txid=%s: input %s is conflicting with previous vote for tx %s\n", __func__,
-                         tx.GetHash().ToString(), in.prevout.ToStringShort(), otherTxHash.ToString());
+                          tx.GetHash().ToString(), in.prevout.ToStringShort(), otherTxHash.ToString());


are this one and the other two whitespace changes below intentionally?

xdustinface

Also, thoughts about IsInstantSendSigningEnabled -> IsInstantSendMempoolSigningEnabled?

src/llmq/quorums_instantsend.cpp

src/llmq/quorums_instantsend.h

src/llmq/quorums_instantsend.cpp

Co-authored-by: dustinface <35775977+xdustinface@users.noreply.github.com>

UdjinM6

I would probably avoid making whitespace changes but it's not a big deal imo and everything else LGTM, so

utACK

xdustinface

utACK with the same opinion about the whitespace stuff + note that the PR description is still mentioning the dropped spork.

* instantsend: refactor input locking into it's own method Signed-off-by: pasta <pasta@dashboost.org> * instantsend: introduce spork 24 `SPORK_24_INSTANTSEND_SIGNING_ENABLED` This spork tells masternodes to refuse to lock transactions in mempool. Only transactions included in a block should be retroactively signed. Signed-off-by: pasta <pasta@dashboost.org> add spork defenition Signed-off-by: pasta <pasta@dashboost.org> * instantsend: refactor `sed -i 's/allowReSigning/fRetroactive/g' src/llmq/*` Signed-off-by: pasta <pasta@dashboost.org> * instantsend: adjust comments Signed-off-by: pasta <pasta@dashboost.org> * instantsend/tests: implement Spork 24 support in tests, and test it's usage Signed-off-by: pasta <pasta@dashboost.org> * fix feature_llmq_is_retroactive.py Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com> * drop Spork 24 and use Spork 2 value 1 as being no mempool signing Signed-off-by: pasta <pasta@dashboost.org> * fix spork check Signed-off-by: pasta <pasta@dashboost.org> * Fix tests Co-authored-by: dustinface <35775977+xdustinface@users.noreply.github.com> * Change comment Co-authored-by: dustinface <35775977+xdustinface@users.noreply.github.com> * IsInstantSendSigningEnabled -> IsInstantSendMempoolSigningEnabled Co-authored-by: dustinface <35775977+xdustinface@users.noreply.github.com> Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com> Co-authored-by: dustinface <35775977+xdustinface@users.noreply.github.com>

PastaPastaPasta added 5 commits March 2, 2021 12:49

instantsend: refactor input locking into it's own method

7e7cd0e

Signed-off-by: pasta <pasta@dashboost.org>

instantsend: refactor `sed -i 's/allowReSigning/fRetroactive/g' src/l…

1419a91

…lmq/*` Signed-off-by: pasta <pasta@dashboost.org>

instantsend: adjust comments

b8e4fcb

Signed-off-by: pasta <pasta@dashboost.org>

instantsend/tests: implement Spork 24 support in tests, and test it's…

67af684

… usage Signed-off-by: pasta <pasta@dashboost.org>

UdjinM6 requested changes Mar 3, 2021

View reviewed changes

src/llmq/quorums_instantsend.cpp Outdated Show resolved Hide resolved

test/functional/feature_llmq_is_retroactive.py Outdated Show resolved Hide resolved

src/llmq/quorums_instantsend.cpp Outdated Show resolved Hide resolved

PastaPastaPasta and others added 3 commits March 3, 2021 17:54

fix feature_llmq_is_retroactive.py

08b6f02

Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>

drop Spork 24 and use Spork 2 value 1 as being no mempool signing

832fc04

Signed-off-by: pasta <pasta@dashboost.org>

Merge branch 'instantsend-signing-spork' of github.com:PastaPastaPast…

90386a1

…a/dash into instantsend-signing-spork

PastaPastaPasta changed the title ~~Implement Spork 24 for InstantSend Signing~~ Implement Spork 2 InstantSend Signing signaling Mar 4, 2021

fix spork check

f04e174

Signed-off-by: pasta <pasta@dashboost.org>

xdustinface suggested changes Mar 5, 2021

View reviewed changes

src/llmq/quorums_instantsend.cpp Outdated Show resolved Hide resolved

src/llmq/quorums_instantsend.h Outdated Show resolved Hide resolved

src/llmq/quorums_instantsend.cpp Outdated Show resolved Hide resolved

PastaPastaPasta and others added 3 commits March 6, 2021 17:25

Fix tests

45c43a2

Co-authored-by: dustinface <35775977+xdustinface@users.noreply.github.com>

Change comment

2a8054a

Co-authored-by: dustinface <35775977+xdustinface@users.noreply.github.com>

IsInstantSendSigningEnabled -> IsInstantSendMempoolSigningEnabled

c81a215

Co-authored-by: dustinface <35775977+xdustinface@users.noreply.github.com>

UdjinM6 added this to the 17 milestone Mar 7, 2021

UdjinM6 approved these changes Mar 7, 2021

View reviewed changes

xdustinface approved these changes Mar 7, 2021

View reviewed changes

UdjinM6 changed the title ~~Implement Spork 2 InstantSend Signing signaling~~ instantsend: Implement Spork 2 Mempool Signing signalling Mar 8, 2021

UdjinM6 merged commit 6da3d5c into dashpay:develop Mar 8, 2021

UdjinM6 mentioned this pull request Mar 9, 2021

bugfix: Add missing return in TrySignInputLocks #4026

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

instantsend: Implement Spork 2 Mempool Signing signalling #4024

instantsend: Implement Spork 2 Mempool Signing signalling #4024

PastaPastaPasta commented Mar 2, 2021 •

edited

Loading

UdjinM6 left a comment

PastaPastaPasta commented Mar 4, 2021

xdustinface left a comment

xdustinface Mar 5, 2021

xdustinface left a comment

UdjinM6 left a comment

xdustinface left a comment

instantsend: Implement Spork 2 Mempool Signing signalling #4024

instantsend: Implement Spork 2 Mempool Signing signalling #4024

Conversation

PastaPastaPasta commented Mar 2, 2021 • edited Loading

UdjinM6 left a comment

Choose a reason for hiding this comment

PastaPastaPasta commented Mar 4, 2021

xdustinface left a comment

Choose a reason for hiding this comment

xdustinface Mar 5, 2021

Choose a reason for hiding this comment

xdustinface left a comment

Choose a reason for hiding this comment

UdjinM6 left a comment

Choose a reason for hiding this comment

xdustinface left a comment

Choose a reason for hiding this comment

PastaPastaPasta commented Mar 2, 2021 •

edited

Loading