Skip to content

Overhaul Qt fee bumper #10449

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.

Sign up for GitHub

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

Jump to bottom
Merged
jonasschnelli merged 2 commits into from
Jun 1, 2017
Merged

Overhaul Qt fee bumper #10449

jonasschnelli merged 2 commits into from
Jun 1, 2017

Conversation

@jonasschnelli
Copy link
Contributor

@jonasschnelli jonasschnelli commented May 24, 2017

This fixes two minor issue:

  • Right now, the context menu "Increase transaction fee" is also enabled for transactions that already have been bumped (fixed in this PR)
  • After bumping a transaction, the original transaction doesn't get properly visually updated (fixed in this PR)

ryanofsky
ryanofsky reviewed May 24, 2017
View reviewed changes
Copy link
Contributor

@ryanofsky ryanofsky left a comment

Choose a reason for hiding this comment

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

Tested ACK 6d7104c. Confirmed menu option is disabled for bumped transactions, and that bumping a transaction now makes it gray and puts the amount in brackets.

@TheBlueMatt
Copy link
Contributor

TheBlueMatt commented May 31, 2017

I thought we were gonna go with not displaying the replaced_by_txid transactions ala TransactionRecord::showTransaction?

@laanwj
Copy link
Member

laanwj commented Jun 1, 2017 •
edited
Loading

These are strictly improvements, utACK 6d7104c.
Agree that hiding the transactions might also be a good idea, but you're the one implementing it (as I remember, hiding transactions is more difficult to get right). Further ideas could always be done later.

@jonasschnelli jonasschnelli merged commit 6d7104c into bitcoin:master Jun 1, 2017
jonasschnelli added a commit that referenced this pull request Jun 1, 2017
@jonasschnelli
Merge #10449: Overhaul Qt fee bumper
64beb13 
6d7104c [Qt] make sure transaction table entry gets updated after bump (Jonas Schnelli)
32325a3 [Qt] hide bump context menu action if tx already has been bumped (Jonas Schnelli)

Tree-SHA512: d3e5991145879b7f6b212d9d9c6f423609dc8e6fa7f6feb7df931691f1dec2acb6ab162c2fb7e758d3ca3f3fb14363df2f50f0e83e83068da5cc7e6de35e69d2
@jtimon
Copy link
Contributor

jtimon commented Jun 1, 2017

This seems to have broken qt/test/test_bitcoin-qt:

==============================================
   Bitcoin Core 0.14.99: src/test-suite.log
==============================================

# TOTAL: 2
# PASS:  1
# SKIP:  0
# XFAIL: 0
# FAIL:  1
# XPASS: 0
# ERROR: 0

.. contents:: :depth: 2

FAIL: qt/test/test_bitcoin-qt
=============================

********* Start testing of URITests *********
Config: Using QtTest library 5.5.1, Qt 5.5.1 (x86_64-little_endian-lp64 shared (dynamic) release build; by GCC 5.3.1 20160413)
PASS   : URITests::initTestCase()
PASS   : URITests::uriTests()
PASS   : URITests::cleanupTestCase()
Totals: 3 passed, 0 failed, 0 skipped, 0 blacklisted
********* Finished testing of URITests *********
********* Start testing of PaymentServerTests *********
Config: Using QtTest library 5.5.1, Qt 5.5.1 (x86_64-little_endian-lp64 shared (dynamic) release build; by GCC 5.3.1 20160413)
PASS   : PaymentServerTests::initTestCase()
QDEBUG : PaymentServerTests::paymentServerTests() PaymentServer::initNetManager: No active proxy server found.
QDEBUG : PaymentServerTests::paymentServerTests() PaymentServer::processPaymentRequest: Secure payment request from  "testmerchant.org"
QWARN  : PaymentServerTests::paymentServerTests() PaymentRequestPlus::getMerchant: Payment request: certificate expired or not yet active:  QSslCertificate("3", "03", "LxHILx+N3qwVoAcCmQ5cyw==", (), ("Expired Test Merchant"), QMap(), QDateTime(2013-02-23 21:26:43.000 UTC Qt::TimeSpec(UTC)), QDateTime(2013-02-24 21:26:43.000 UTC Qt::TimeSpec(UTC)))
QDEBUG : PaymentServerTests::paymentServerTests() PaymentServer::processPaymentRequest: Insecure payment request to  "1EcqLEpyu3zY7T5QU7RLrHEQH134KeGPXR"
QWARN  : PaymentServerTests::paymentServerTests() PaymentRequestPlus::getMerchant: Payment request: certificate expired or not yet active:  QSslCertificate("3", "03", "LxHILx+N3qwVoAcCmQ5cyw==", (), ("Expired Test Merchant"), QMap(), QDateTime(2013-02-23 21:26:43.000 UTC Qt::TimeSpec(UTC)), QDateTime(2013-02-24 21:26:43.000 UTC Qt::TimeSpec(UTC)))
QDEBUG : PaymentServerTests::paymentServerTests() PaymentServer::processPaymentRequest: Secure payment request from  "testmerchant8.org"
QWARN  : PaymentServerTests::paymentServerTests() PaymentRequestPlus::getMerchant: Payment request: certificate expired or not yet active:  QSslCertificate("3", "06", "MiZaQ+g9lSHZGuHWkXZG+g==", (), ("Payment Request Intermediate 5"), QMap(), QDateTime(2013-02-23 22:59:51.000 UTC Qt::TimeSpec(UTC)), QDateTime(2013-02-24 22:59:51.000 UTC Qt::TimeSpec(UTC)))
QDEBUG : PaymentServerTests::paymentServerTests() PaymentServer::processPaymentRequest: Insecure payment request to  "1EcqLEpyu3zY7T5QU7RLrHEQH134KeGPXR"
QWARN  : PaymentServerTests::paymentServerTests() PaymentRequestPlus::getMerchant: Payment request: certificate expired or not yet active:  QSslCertificate("3", "06", "MiZaQ+g9lSHZGuHWkXZG+g==", (), ("Payment Request Intermediate 5"), QMap(), QDateTime(2013-02-23 22:59:51.000 UTC Qt::TimeSpec(UTC)), QDateTime(2013-02-24 22:59:51.000 UTC Qt::TimeSpec(UTC)))
QWARN  : PaymentServerTests::paymentServerTests() PaymentRequestPlus::getMerchant: SSL error:  certificate signature failure
QDEBUG : PaymentServerTests::paymentServerTests() PaymentServer::processPaymentRequest: Insecure payment request to  "1EcqLEpyu3zY7T5QU7RLrHEQH134KeGPXR"
QWARN  : PaymentServerTests::paymentServerTests() PaymentRequestPlus::getMerchant: SSL error:  certificate signature failure
QWARN  : PaymentServerTests::paymentServerTests() PaymentRequestPlus::getMerchant: SSL error:  unable to get local issuer certificate
QDEBUG : PaymentServerTests::paymentServerTests() PaymentServer::processPaymentRequest: Insecure payment request to  "1EcqLEpyu3zY7T5QU7RLrHEQH134KeGPXR"
QWARN  : PaymentServerTests::paymentServerTests() PaymentRequestPlus::getMerchant: SSL error:  unable to get local issuer certificate
QWARN  : PaymentServerTests::paymentServerTests() "PaymentServer::verifyNetwork: Payment request network \"test\" doesn't match client network \"main\"."
QWARN  : PaymentServerTests::paymentServerTests() "PaymentServer::verifyExpired: Payment request expired \"1970-01-01 00:00:01\"."
QWARN  : PaymentServerTests::paymentServerTests() "PaymentServer::verifyExpired: Payment request expired \"1970-01-01 00:00:00\"."
QWARN  : PaymentServerTests::paymentServerTests() "PaymentServer::verifySize: Payment request too large (50001 bytes, allowed 50000 bytes)."
QWARN  : PaymentServerTests::paymentServerTests() "PaymentServer::verifyAmount: Payment request amount out of allowed range (2100000100000000, allowed 0 - 2100000000000000)."
PASS   : PaymentServerTests::paymentServerTests()
PASS   : PaymentServerTests::cleanupTestCase()
Totals: 3 passed, 0 failed, 0 skipped, 0 blacklisted
********* Finished testing of PaymentServerTests *********
********* Start testing of RPCNestedTests *********
Config: Using QtTest library 5.5.1, Qt 5.5.1 (x86_64-little_endian-lp64 shared (dynamic) release build; by GCC 5.3.1 20160413)
PASS   : RPCNestedTests::initTestCase()
PASS   : RPCNestedTests::rpcNestedTests()
PASS   : RPCNestedTests::cleanupTestCase()
Totals: 3 passed, 0 failed, 0 skipped, 0 blacklisted
********* Finished testing of RPCNestedTests *********
********* Start testing of CompatTests *********
Config: Using QtTest library 5.5.1, Qt 5.5.1 (x86_64-little_endian-lp64 shared (dynamic) release build; by GCC 5.3.1 20160413)
PASS   : CompatTests::initTestCase()
PASS   : CompatTests::bswapTests()
PASS   : CompatTests::cleanupTestCase()
Totals: 3 passed, 0 failed, 0 skipped, 0 blacklisted
********* Finished testing of CompatTests *********
********* Start testing of WalletTests *********
Config: Using QtTest library 5.5.1, Qt 5.5.1 (x86_64-little_endian-lp64 shared (dynamic) release build; by GCC 5.3.1 20160413)
PASS   : WalletTests::initTestCase()
QDEBUG : WalletTests::walletTests() TransactionTablePriv::refreshWallet
QWARN  : WalletTests::walletTests() This plugin does not support propagateSizeHints()
QDEBUG : WalletTests::walletTests() "NotifyTransactionChanged: 6ebadef409a5733dffa9dfd693740f752a1df5e0330962b40cb8c93a67d11f29 status= 0"
QDEBUG : WalletTests::walletTests() "NotifyTransactionChanged: 8bb447619a7bd0be53f46491584cf906564ab576082d3bc97cf321f436b8aa5e status= 1"
QDEBUG : WalletTests::walletTests() "NotifyAddressBookChanged: mfWxJ45yp2SFn7UciZyNpvDKrzbhyfKrY8  isMine=0 purpose=send status=0"
QDEBUG : WalletTests::walletTests() "TransactionTablePriv::updateWallet: 6ebadef409a5733dffa9dfd693740f752a1df5e0330962b40cb8c93a67d11f29 0"
QDEBUG : WalletTests::walletTests() "    inModel=0 Index=13-13 showTransaction=1 derivedStatus=0"
QDEBUG : WalletTests::walletTests() "TransactionTablePriv::updateWallet: 8bb447619a7bd0be53f46491584cf906564ab576082d3bc97cf321f436b8aa5e 1"
QDEBUG : WalletTests::walletTests() "    inModel=1 Index=35-36 showTransaction=1 derivedStatus=1"
QWARN  : WalletTests::walletTests() This plugin does not support propagateSizeHints()
QDEBUG : WalletTests::walletTests() "NotifyTransactionChanged: 92b5e72a4c3be337996d705b3d65ff1d91d25c4fe0afc7e79a1b2879dc54b383 status= 0"
QDEBUG : WalletTests::walletTests() "NotifyTransactionChanged: 97b70d1ea6e61f65b42b52c0859581067536929ef4e97efbd2d6039cb92a59c0 status= 1"
QDEBUG : WalletTests::walletTests() "TransactionTablePriv::updateWallet: 92b5e72a4c3be337996d705b3d65ff1d91d25c4fe0afc7e79a1b2879dc54b383 0"
QDEBUG : WalletTests::walletTests() "    inModel=0 Index=53-53 showTransaction=1 derivedStatus=0"
QDEBUG : WalletTests::walletTests() "TransactionTablePriv::updateWallet: 97b70d1ea6e61f65b42b52c0859581067536929ef4e97efbd2d6039cb92a59c0 1"
QDEBUG : WalletTests::walletTests() "    inModel=1 Index=82-83 showTransaction=1 derivedStatus=1"
QWARN  : WalletTests::walletTests() This plugin does not support raise()
QWARN  : WalletTests::walletTests() This plugin does not support grabbing the keyboard
QWARN  : WalletTests::walletTests() This plugin does not support propagateSizeHints()
QWARN  : WalletTests::walletTests() This plugin does not support raise()
QWARN  : WalletTests::walletTests() This plugin does not support grabbing the keyboard
QWARN  : WalletTests::walletTests() This plugin does not support propagateSizeHints()
QWARN  : WalletTests::walletTests() This plugin does not support raise()
QWARN  : WalletTests::walletTests() This plugin does not support grabbing the keyboard
QWARN  : WalletTests::walletTests() This plugin does not support propagateSizeHints()
QDEBUG : WalletTests::walletTests() "NotifyTransactionChanged: 1a9b91b3b7497a87a089fb1744bac71520e329b4082aafac87b2436fc44fa846 status= 0"
QDEBUG : WalletTests::walletTests() "NotifyTransactionChanged: 97b70d1ea6e61f65b42b52c0859581067536929ef4e97efbd2d6039cb92a59c0 status= 1"
QDEBUG : WalletTests::walletTests() "NotifyTransactionChanged: 92b5e72a4c3be337996d705b3d65ff1d91d25c4fe0afc7e79a1b2879dc54b383 status= 1"
QDEBUG : WalletTests::walletTests() "TransactionTablePriv::updateWallet: 92b5e72a4c3be337996d705b3d65ff1d91d25c4fe0afc7e79a1b2879dc54b383 1"
QDEBUG : WalletTests::walletTests() "    inModel=1 Index=53-54 showTransaction=1 derivedStatus=1"
QWARN  : WalletTests::walletTests() This plugin does not support raise()
QWARN  : WalletTests::walletTests() This plugin does not support grabbing the keyboard
FAIL!  : WalletTests::walletTests() Compared values are not the same
   Actual   (action->isEnabled()): 0
   Expected (!expectDisabled)    : 1
   Loc: [qt/test/wallettests.cpp(117)]
PASS   : WalletTests::cleanupTestCase()
Totals: 2 passed, 1 failed, 0 skipped, 0 blacklisted
********* Finished testing of WalletTests *********
FAIL qt/test/test_bitcoin-qt (exit status: 1)

ryanofsky added a commit to ryanofsky/bitcoin that referenced this pull request Jun 1, 2017
@ryanofsky
Fix bumpfee test after bitcoin#10449
8906a9a 
Failure reported by Jorge Timón <[email protected]>
bitcoin#10449 (comment)
@ryanofsky ryanofsky mentioned this pull request Jun 1, 2017
Merged
sipa added a commit that referenced this pull request Jun 1, 2017
@sipa
Merge #10506: Fix bumpfee test after #10449
7cc2c67 
8906a9a Fix bumpfee test after #10449 (Russell Yanofsky)

Tree-SHA512: 0838c7696499baf0fb5ee6edf0b081752d6c37578360a7f24a7e9c700598cbc14ff95826f2f5124cca805d2609470a052bc7309211874b13be7ac1ff9e911a34
HashUnlimited pushed a commit to chaincoin/chaincoin that referenced this pull request Mar 6, 2018
@ryanofsky
Fix bumpfee test after bitcoin#10449
2ebbebf 
Failure reported by Jorge Timón <[email protected]>
bitcoin#10449 (comment)
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@ryanofsky ryanofsky ryanofsky left review comments

Assignees

No one assigned

Labels

GUI

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@jonasschnelli @TheBlueMatt @laanwj @jtimon @ryanofsky