[Qt] attribute replaceable (RBF) transactions #7817

jonasschnelli · 2016-04-05T14:14:04Z

This attributes transactiontable entries (one or more per wtx) if the transactions signals opt-in-RBF.
A light orange color together with a new icon (for "incoming" and "outgoing" transactions).

paveljanik · 2016-04-05T17:21:31Z

src/qt/transactionrecord.cpp

@@ -7,6 +7,7 @@
 #include "base58.h"
 #include "consensus/consensus.h"
 #include "main.h"
+#include "policy/rbf.h"


Do you need this here?

Right. This is not required. Fixed.

maflcko · 2016-04-05T18:21:57Z

Concept ACK

~~Note: Please don't retrigger travis, as I reported the issue to them.~~

paveljanik · 2016-04-05T18:36:41Z

src/qt/transactiondesc.cpp

@@ -39,7 +39,7 @@ QString TransactionDesc::FormatTxStatus(const CWalletTx& wtx)
        else if (GetAdjustedTime() - wtx.nTimeReceived > 2 * 60 && wtx.GetRequestCount() == 0)
            return tr("%1/offline").arg(nDepth);
        else if (nDepth == 0)
-            return tr("0/unconfirmed, %1").arg((wtx.InMempool() ? tr("in memory pool") : tr("not in memory pool"))) + (wtx.isAbandoned() ? ", "+tr("abandoned") : "");
+            return tr("0/unconfirmed, %1").arg((wtx.InMempool() ? tr("in memory pool") : tr("not in memory pool"))) + (wtx.signalsOptInRBF() ? ", "+tr("replaceable") : "") + (wtx.isAbandoned() ? ", "+tr("abandoned") : "");


We will present transactions signalling opt-in RBF as "replaceable". Users will ask, what it means.

Edit: Greg asked the same question in other PR. We should probably clarify this. What about this as a topic for Thu meeting?

paveljanik · 2016-04-05T18:37:15Z

Concept ACK

btcdrak · 2016-05-17T21:11:41Z

Travis needs a restart now.

petertodd · 2016-05-17T21:18:46Z

Concept ACK for outgoing.

Concept NACK for incoming - I'm worried about giving a false sense of security given the multitude of other ways of doublespending.

luke-jr · 2016-05-17T22:08:48Z

Mostly agreed with @petertodd

Probably the new icon/colour should be set for all unconfirmed transactions signed by a third party, whether incoming or outgoing.

paveljanik · 2016-05-18T05:09:46Z

@petertodd You mean that marking "incoming" tx signalling RBF with this icon can bring false sense of security to non-RBF transactions not marked with this icon? Or? Can you compare this situation with the current status quo?

petertodd · 2016-05-20T08:18:51Z

@paveljanik So the status quo, is that all incoming unconfirmed txs look the same; I'm concerned that showing some types of unconfirmed incoming txs differently than others in UI's intended for general users gives the false impression that Bitcoin Core can reliably tell you about the risk of any particular unconfirmed incoming txs; expert-level UI's like the RPC interface are much less of a concern to me.

paveljanik · 2016-05-20T08:23:04Z

@petertodd In such case, we should not show Unconfirmed at all, add a number input item defaulting to 6 into options and only show transactions with such number of confirmations.

paveljanik · 2016-05-20T08:25:42Z

As all unconfirmed txs are marked with ? in the UI, I think this is safe as long as there is a ? on RBF signalling txs.

petertodd · 2016-05-20T08:25:43Z

@paveljanik But unconfirmed is a state that I think users understand just fine: "Someone intends to send me a transaction, and if they're honest it'll probably confirm (eventually)." There's nothing wrong with showing that.

paveljanik · 2016-05-20T08:55:10Z

Interesting Travis failures - qInitResources_bitcoin not found. The log contains:

  GEN      qt/qrc_bitcoin.cpp
RCC: Error in 'qt/bitcoin.qrc': Cannot find file 'res/icons/transaction_replaceable.png'

laanwj · 2016-06-09T06:35:00Z

Probably you need to add the file in a Makefile so that it is included in the distribution

jonasschnelli · 2016-06-09T09:26:37Z

Fixed nit and added missing file to Makefile.

jonasschnelli · 2016-06-09T09:27:56Z

@petertodd @luke-jr :
What about keeping the icon but not the orangish color for incoming RBF transactions?

gmaxwell · 2016-06-15T10:29:43Z

@petertodd ?

I like it with just the icon myself.

petertodd · 2016-06-15T16:54:53Z

@gmaxwell @jonasschnelli Eh, fine, go with that.

laanwj · 2016-06-20T12:59:51Z

utACK 7a30ed6

(also agree on removing the orange color but keeping the icon)

maflcko · 2016-06-20T14:30:20Z

Looks like getting rid of the orange color is preferred.

@jonasschnelli Mind to take a look?

Also the nit still holds: #7817 (comment)

laanwj · 2016-06-21T09:51:14Z

Removing 0.13 milestone, this missed the feature freeze.

btcdrak · 2016-08-19T16:48:42Z

needs rebase

jonasschnelli · 2016-08-19T16:49:46Z

I think we should first focus on #8456 (before we continue with the GUI level).

luke-jr · 2016-09-10T06:41:50Z

@jonasschnelli AFAICT it still has the same problem with just the icon. It gives a false sense of security; there is literally no rational distinction between them for incoming transactions. But maybe I'm missing some use case?

paveljanik · 2016-12-10T19:54:18Z

Rebase needed.

jonasschnelli · 2017-01-06T09:49:02Z

Closing for now due to controversy and inactivity...

@conscott

… entries 870bd4c Update functional RBF test to check replaceable flag (dexX7) 820d31f Add "bip125-replaceable" flag to mempool RPCs (dexX7) Pull request description: This pull request adds a flag "bip125-replaceable" to the mempool RPCs getrawmempool, getmempoolentry, getmempoolancestors and getmempooldescendants, which indicates whether an unconfirmed transaction might be replaced. Initially the flag was added to the raw transaction RPCs, but thanks to @conscott, it was moved to the mempool RPCs, which actually have access to the mempool. ~~This pull request adds a flag "bip125-replaceable" to the RPCs "getrawtransaction" and "decoderawtransaction", which indicates, whether a transaction signals BIP 125 replaceability.~~ There was some discussion in #7817, whether showing replaceability in the UI could lead to the false assumption that transactions that don't signal BIP 125 are truely non-replaceable, but given that this PR tackles the raw transaction interface, which is a rather low level tool, I believe having this extra piece of information isn't bad. Tree-SHA512: 1f5511957af2c20a9a6c79d80a335c3be37a2402dbf829c40cceaa01a24868eab81a9c1cdb0b3d77198fa3bb82799e3540a5c0ce7f35bbac80d73f7133ff7cbc

@conscott

…mempool entries 870bd4c Update functional RBF test to check replaceable flag (dexX7) 820d31f Add "bip125-replaceable" flag to mempool RPCs (dexX7) Pull request description: This pull request adds a flag "bip125-replaceable" to the mempool RPCs getrawmempool, getmempoolentry, getmempoolancestors and getmempooldescendants, which indicates whether an unconfirmed transaction might be replaced. Initially the flag was added to the raw transaction RPCs, but thanks to @conscott, it was moved to the mempool RPCs, which actually have access to the mempool. ~~This pull request adds a flag "bip125-replaceable" to the RPCs "getrawtransaction" and "decoderawtransaction", which indicates, whether a transaction signals BIP 125 replaceability.~~ There was some discussion in bitcoin#7817, whether showing replaceability in the UI could lead to the false assumption that transactions that don't signal BIP 125 are truely non-replaceable, but given that this PR tackles the raw transaction interface, which is a rather low level tool, I believe having this extra piece of information isn't bad. Tree-SHA512: 1f5511957af2c20a9a6c79d80a335c3be37a2402dbf829c40cceaa01a24868eab81a9c1cdb0b3d77198fa3bb82799e3540a5c0ce7f35bbac80d73f7133ff7cbc # Conflicts: # src/rpc/blockchain.cpp # test/functional/feature_rbf.py

@conscott

…mempool entries 870bd4c Update functional RBF test to check replaceable flag (dexX7) 820d31f Add "bip125-replaceable" flag to mempool RPCs (dexX7) Pull request description: This pull request adds a flag "bip125-replaceable" to the mempool RPCs getrawmempool, getmempoolentry, getmempoolancestors and getmempooldescendants, which indicates whether an unconfirmed transaction might be replaced. Initially the flag was added to the raw transaction RPCs, but thanks to @conscott, it was moved to the mempool RPCs, which actually have access to the mempool. ~~This pull request adds a flag "bip125-replaceable" to the RPCs "getrawtransaction" and "decoderawtransaction", which indicates, whether a transaction signals BIP 125 replaceability.~~ There was some discussion in bitcoin#7817, whether showing replaceability in the UI could lead to the false assumption that transactions that don't signal BIP 125 are truely non-replaceable, but given that this PR tackles the raw transaction interface, which is a rather low level tool, I believe having this extra piece of information isn't bad. Tree-SHA512: 1f5511957af2c20a9a6c79d80a335c3be37a2402dbf829c40cceaa01a24868eab81a9c1cdb0b3d77198fa3bb82799e3540a5c0ce7f35bbac80d73f7133ff7cbc # Conflicts: # src/rpc/blockchain.cpp # test/functional/feature_rbf.py

@conscott

…mempool entries 870bd4c Update functional RBF test to check replaceable flag (dexX7) 820d31f Add "bip125-replaceable" flag to mempool RPCs (dexX7) Pull request description: This pull request adds a flag "bip125-replaceable" to the mempool RPCs getrawmempool, getmempoolentry, getmempoolancestors and getmempooldescendants, which indicates whether an unconfirmed transaction might be replaced. Initially the flag was added to the raw transaction RPCs, but thanks to @conscott, it was moved to the mempool RPCs, which actually have access to the mempool. ~~This pull request adds a flag "bip125-replaceable" to the RPCs "getrawtransaction" and "decoderawtransaction", which indicates, whether a transaction signals BIP 125 replaceability.~~ There was some discussion in bitcoin#7817, whether showing replaceability in the UI could lead to the false assumption that transactions that don't signal BIP 125 are truely non-replaceable, but given that this PR tackles the raw transaction interface, which is a rather low level tool, I believe having this extra piece of information isn't bad. Tree-SHA512: 1f5511957af2c20a9a6c79d80a335c3be37a2402dbf829c40cceaa01a24868eab81a9c1cdb0b3d77198fa3bb82799e3540a5c0ce7f35bbac80d73f7133ff7cbc # Conflicts: # src/rpc/blockchain.cpp # test/functional/feature_rbf.py

jonasschnelli added the GUI label Apr 5, 2016

paveljanik reviewed Apr 5, 2016
View reviewed changes

maflcko mentioned this pull request Apr 5, 2016

Travis sometimes misses commits/pulls travis-ci/travis-ci#5863

Closed

paveljanik reviewed Apr 5, 2016
View reviewed changes

jonasschnelli force-pushed the 2016/04/qt_rbf branch from 4719671 to 6a88002 Compare April 28, 2016 09:34

petertodd mentioned this pull request May 20, 2016

Support fee bumping in the UI spesmilo/electrum#1307

Closed

jonasschnelli mentioned this pull request Jun 9, 2016

[Qt] Add simple opt-in-RBF support #8182

Closed

jonasschnelli force-pushed the 2016/04/qt_rbf branch from 6a88002 to ab9fc41 Compare June 9, 2016 09:24

jonasschnelli added this to the 0.13.0 milestone Jun 9, 2016

[Qt] attribute replaceable (RBF) transactions

7a30ed6

jonasschnelli force-pushed the 2016/04/qt_rbf branch from ab9fc41 to 7a30ed6 Compare June 10, 2016 07:52

laanwj removed this from the 0.13.0 milestone Jun 21, 2016

laanwj mentioned this pull request Jun 21, 2016

[Qt] show conflicts of unconfirmed transactions in the UI #7826

Closed

jonasschnelli closed this Jan 6, 2017

dexX7 mentioned this pull request Mar 12, 2018

Show "bip125-replaceable" flag, when retrieving mempool entries #12676

Merged

bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Qt] attribute replaceable (RBF) transactions #7817

[Qt] attribute replaceable (RBF) transactions #7817

jonasschnelli commented Apr 5, 2016

paveljanik Apr 5, 2016

jonasschnelli Apr 6, 2016

maflcko commented Apr 5, 2016

paveljanik Apr 5, 2016

paveljanik commented Apr 5, 2016

btcdrak commented May 17, 2016

petertodd commented May 17, 2016

luke-jr commented May 17, 2016

paveljanik commented May 18, 2016 •

edited

petertodd commented May 20, 2016

paveljanik commented May 20, 2016

paveljanik commented May 20, 2016

petertodd commented May 20, 2016

paveljanik commented May 20, 2016

laanwj commented Jun 9, 2016 •

edited

jonasschnelli commented Jun 9, 2016

jonasschnelli commented Jun 9, 2016

gmaxwell commented Jun 15, 2016

petertodd commented Jun 15, 2016

laanwj commented Jun 20, 2016 •

edited

maflcko commented Jun 20, 2016

laanwj commented Jun 21, 2016

btcdrak commented Aug 19, 2016

jonasschnelli commented Aug 19, 2016

luke-jr commented Sep 10, 2016

paveljanik commented Dec 10, 2016

jonasschnelli commented Jan 6, 2017

[Qt] attribute replaceable (RBF) transactions #7817

[Qt] attribute replaceable (RBF) transactions #7817

Conversation

jonasschnelli commented Apr 5, 2016

paveljanik Apr 5, 2016

Choose a reason for hiding this comment

jonasschnelli Apr 6, 2016

Choose a reason for hiding this comment

maflcko commented Apr 5, 2016

paveljanik Apr 5, 2016

Choose a reason for hiding this comment

paveljanik commented Apr 5, 2016

btcdrak commented May 17, 2016

petertodd commented May 17, 2016

luke-jr commented May 17, 2016

paveljanik commented May 18, 2016 • edited

petertodd commented May 20, 2016

paveljanik commented May 20, 2016

paveljanik commented May 20, 2016

petertodd commented May 20, 2016

paveljanik commented May 20, 2016

laanwj commented Jun 9, 2016 • edited

jonasschnelli commented Jun 9, 2016

jonasschnelli commented Jun 9, 2016

gmaxwell commented Jun 15, 2016

petertodd commented Jun 15, 2016

laanwj commented Jun 20, 2016 • edited

maflcko commented Jun 20, 2016

laanwj commented Jun 21, 2016

btcdrak commented Aug 19, 2016

jonasschnelli commented Aug 19, 2016

luke-jr commented Sep 10, 2016

paveljanik commented Dec 10, 2016

jonasschnelli commented Jan 6, 2017

paveljanik commented May 18, 2016 •

edited

laanwj commented Jun 9, 2016 •

edited

laanwj commented Jun 20, 2016 •

edited