Remove legacy InstantSend code #3020

codablock · 2019-07-07T18:06:54Z

This PR removes all legacy InstantSend code, including UI and RPC stuff.
See individual commits for details.

UdjinM6

Looks good 👍

pls see few suggestions below

src/protocol.cpp

src/qt/res/css/crownium.css

src/qt/transactiondesc.cpp

UdjinM6

Looks good 👍

slightly tested (client-side only) ACK

PastaPastaPasta

Shouldn't spork 2 and 3 be removed entirely?

codablock · 2019-07-08T13:39:17Z

@PastaPastaPasta Nope, spork 5, 6 and 20 are removed instead. Spork 20 was only meant to switch to the new system, but spork 2 is still meant to signal if InstantSend is enabled or not. But...I just realized that we have to clarify with the mobile team if they actually handle sporks this way...

codablock · 2019-07-08T14:29:03Z

Just clarified with the mobile team that spork20 can't be easily removed as they internally still depend on it. We can remove spork20 in a few months however. I've reverted removal of spork20 and squashed that into the original commit.

UdjinM6 · 2019-07-08T14:35:35Z

Should also fix tests now

codablock · 2019-07-08T14:37:47Z

Not sure I understand...you assume that tests will fail?

UdjinM6 · 2019-07-08T14:40:13Z

Oh, you only reverted changes related to spork20 existence, not the logic behind it, I see.

codablock · 2019-07-08T14:41:53Z

Exactly, this is really just to ensure that the spork is propagated to clients/nodes that still need it.

UdjinM6

ok, utACK :)

UdjinM6 · 2019-07-08T15:16:45Z

needs rebase

src/protocol.cpp

src/qt/transactiondesc.cpp

PastaPastaPasta · 2019-07-08T16:09:41Z

src/qt/walletmodel.cpp

if( -> if (

Out of scope of this PR...wallet.cpp is generally loaded with style infringements, but fixing them would lead to many merge conflicts in backports.

Ah sorry, this is walletmodel.cpp and not wallet.cpp

PastaPastaPasta · 2019-07-08T16:13:02Z

src/wallet/rpcwallet.cpp

I believe these should use emplace_back instead of push_back (out of scope for this PR)

Yepp, out of scope

src/protocol.h

codablock · 2019-07-08T17:31:23Z

Rebased and force-pushed.

codablock · 2019-07-08T17:41:52Z

FYI, this should help re-reviewing after the force-push:

$ function gfd() {
        local fp1=$(git merge-base --fork-point develop $1)
        local fp2=$(git merge-base --fork-point develop $2)
        echo fp1=$fp1
        echo fp2=$fp2
        diff --color=always -u -I'^[^-+]' <(git diff $fp1..$1) <(git diff $fp2..$2)
}
$ gfd b18ff68fc6c9289a8e08db29cad9228904c3e350 38d1aeabcc3133c00971f2ee0a95a2b4e6e52da2

This will show a diff of the diffs and thus filters out all the noise that you usually have when looking at a diff like in the "force-pushed" links created by Github. It however requires you to have the old commit locally, e.g. because you pulled the PR or my branch locally.

src/qt/transactiondesc.cpp

PastaPastaPasta

utACK, all commits looks good. I want to build locally and test this tonight before we merge

UdjinM6 · 2019-07-09T06:12:01Z

Needs rebase again.

This removes the need to carefully maintain ppszTypeName, which required correct order and also did not allow to permanently remove old message types. To get the command name for an INV type, GetCommandInternal uses a switch which needs to be maintained from now on. The way this is implemented also resembles the way it is implemented in Bitcoin today, but it's not identical. The original PR that introduced the switch case in Bitcoin was part of the Segwit changes and thus never got backported. I decided to implement it in a slightly different way that avoids throwing exceptions when an unknown INV type is encountered. IsKnownType will now also leverage GetCommandInternal() to figure out if the INV type is known locally. This has the side effect of old/legacy message types to return false from now on. We will depend on this side effect in later commits when we remove legacy InstantSend code.

When we receive an IX message, we simply treat it as a regular TX and relay it as such. We'll however still request IX messages when they are announced to us. We can't simply revert to requesting TX messages in this case as it might result in the other peer not answering due to the TX not being in mapRelay yet. We should at some point in the future completely drop handling of IX messages instead.

This should have no influence on mainnet as these sporks are actually set there. This will however affect regtest, which shouldn't have LLMQ based InstantSend enabled by default.

These were only testing legacy InstantSend

…stantSend"

This was for fUseInstantSend which is not present anymore since rebase

codablock · 2019-07-09T07:08:49Z

Rebased and force-pushed. Also re-added SPORK_16_INSTANTSEND_AUTOLOCKS as it is checked by a few mobile clients to decide on necessary fees.

UdjinM6

re-utACK

PastaPastaPasta

re-utACK

ad4b164 chore: drop deprecated RPC `instantsendtoaddress` (Kittywhiskers Van Gogh) Pull request description: ## Additional Information * `instantsendtoaddress` was deprecated in [dash#3020](#3020), which was included in Dash Core v0.15. ## Breaking Changes Deprecated RPC will no longer be available. ## Checklist - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)** - [x] I have added or updated relevant unit/integration/functional/e2e tests **(note: N/A)** - [x] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: PastaPastaPasta: utACK ad4b164 Tree-SHA512: 62f63d7eacdcddb31987eea0e55b033d9d89c0861ec096d442226f6ae530b2850be465ebfc24501763cb3c30444b36fbc3f02d1f355b2b58a174bdb833f6c6cd

codablock added this to the 14.1 milestone Jul 7, 2019

UdjinM6 added P2P Some notable changes on p2p level RPC Some notable changes to RPC params/behaviour/descriptions labels Jul 7, 2019

UdjinM6 requested changes Jul 8, 2019

View reviewed changes

src/protocol.cpp Outdated Show resolved Hide resolved

src/qt/res/css/crownium.css Outdated Show resolved Hide resolved

src/qt/transactiondesc.cpp Outdated Show resolved Hide resolved

UdjinM6 previously approved these changes Jul 8, 2019

View reviewed changes

PastaPastaPasta reviewed Jul 8, 2019

View reviewed changes

codablock dismissed UdjinM6’s stale review via 78e7df7 July 8, 2019 14:28

codablock force-pushed the pr_remove_legacy_instantsend branch from 7a8d80f to 78e7df7 Compare July 8, 2019 14:28

UdjinM6 previously approved these changes Jul 8, 2019

View reviewed changes

PastaPastaPasta reviewed Jul 8, 2019

View reviewed changes

codablock dismissed UdjinM6’s stale review via b18ff68 July 8, 2019 17:31

codablock force-pushed the pr_remove_legacy_instantsend branch from 78e7df7 to b18ff68 Compare July 8, 2019 17:31

codablock force-pushed the pr_remove_legacy_instantsend branch from b18ff68 to 38d1aea Compare July 8, 2019 17:38

PastaPastaPasta reviewed Jul 8, 2019

View reviewed changes

src/qt/transactiondesc.cpp Outdated Show resolved Hide resolved

PastaPastaPasta previously approved these changes Jul 8, 2019

View reviewed changes

codablock dismissed PastaPastaPasta’s stale review via 56e52a9 July 8, 2019 19:06

UdjinM6 mentioned this pull request Jul 8, 2019

Backport #8694: Basic multiwallet support #3022

Merged

codablock added 3 commits July 9, 2019 09:00

Remove IsNewInstantSendEnabled() and only use IsInstantSendEnabled()

f438a57

codablock and others added 17 commits July 9, 2019 09:00

Remove legacy InstantSend from GUI

56ab8ad

Remove InstantSend from Bitcoin/Dash URIs

103d879

Remove legacy InstantSend from RPC commands

53e35a0

Remove legacy InstantSend from wallet

f459686

Remove legacy instantsend.h include

da9a750

Remove legacy InstantSend from validation code

7ebe1e3

Completely remove remaining legacy InstantSend code

73fc3dd

Remove now unused spork

5d9891f

Make spork2 and spork3 disabled by default

99e417a

This should have no influence on mainnet as these sporks are actually set there. This will however affect regtest, which shouldn't have LLMQ based InstantSend enabled by default.

Remove now obsolete auto IS tests

7bdc52e

Remove instantsend tests from dip3-deterministicmns.py

6ee6447

These were only testing legacy InstantSend

Fix InstantSend related test failures

fbcc29a

Fix .QCheckBox#checkUsePrivateSend styling a bit

3538e3f

s/TXLEGACYLOCKREQUEST/LEGACYTXLOCKREQUEST/

1e4385b

Revert "verified via InstantSend" back to "verified via LLMQ based In…

f6f2a8f

…stantSend"

Use cmd == nullptr instead of !cmd

4192aab

Remove last parameter from AvailableCoins call

8a2e8e7

This was for fUseInstantSend which is not present anymore since rebase

codablock force-pushed the pr_remove_legacy_instantsend branch from 56e52a9 to 8a2e8e7 Compare July 9, 2019 07:07

UdjinM6 approved these changes Jul 9, 2019

View reviewed changes

PastaPastaPasta approved these changes Jul 9, 2019

View reviewed changes

UdjinM6 merged commit 2f21e55 into dashpay:develop Jul 9, 2019

thephez mentioned this pull request Aug 8, 2019

V0.14.1 P2P deprecations dash-docs/dash-docs#144

Merged

kwvg mentioned this pull request May 25, 2025

chore: drop deprecated RPC instantsendtoaddress #6686

Merged

5 tasks

Remove legacy InstantSend code #3020

Remove legacy InstantSend code #3020

Uh oh!

Conversation

codablock commented Jul 7, 2019

Uh oh!

UdjinM6 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

UdjinM6 left a comment

Choose a reason for hiding this comment

Uh oh!

PastaPastaPasta left a comment

Choose a reason for hiding this comment

Uh oh!

codablock commented Jul 8, 2019

Uh oh!

codablock commented Jul 8, 2019

Uh oh!

UdjinM6 commented Jul 8, 2019

Uh oh!

codablock commented Jul 8, 2019

Uh oh!

UdjinM6 commented Jul 8, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codablock commented Jul 8, 2019

Uh oh!

UdjinM6 left a comment

Choose a reason for hiding this comment

Uh oh!

UdjinM6 commented Jul 8, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

PastaPastaPasta Jul 8, 2019

Choose a reason for hiding this comment

Uh oh!

codablock Jul 8, 2019

Choose a reason for hiding this comment

Uh oh!

codablock Jul 8, 2019

Choose a reason for hiding this comment

Uh oh!

PastaPastaPasta Jul 8, 2019

Choose a reason for hiding this comment

Uh oh!

codablock Jul 8, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codablock commented Jul 8, 2019

Uh oh!

codablock commented Jul 8, 2019

Uh oh!

Uh oh!

PastaPastaPasta left a comment

Choose a reason for hiding this comment

Uh oh!

UdjinM6 commented Jul 9, 2019

Uh oh!

codablock commented Jul 9, 2019

Uh oh!

UdjinM6 left a comment

Choose a reason for hiding this comment

Uh oh!

PastaPastaPasta left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

UdjinM6 commented Jul 8, 2019 •

edited

Loading