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

PR24067 follow-ups #538

Closed
maflcko opened this issue Jan 26, 2022 · 1 comment · Fixed by #552
Closed

PR24067 follow-ups #538

maflcko opened this issue Jan 26, 2022 · 1 comment · Fixed by #552
Labels

Comments

@maflcko
Copy link
Contributor

maflcko commented Jan 26, 2022

Some follow up ideas for bitcoin/bitcoin#24067

  • Remove now unused open_for
  • Use C++11 member initializers
  • Fix whitespace

Useful skills:

  • Building Bitcoin Core GUI
  • C++

Want to work on this issue?

For guidance on contributing, please read CONTRIBUTING.md before opening your pull request.

@maflcko maflcko added the good first issue Good for newcomers label Jan 26, 2022
@maflcko
Copy link
Contributor Author

maflcko commented Jan 26, 2022

This is the diff I drafted, but it needs to be split into separate commits, ideally:

diff --git a/src/qt/transactiondesc.cpp b/src/qt/transactiondesc.cpp
index be5851d627..4f18c58b20 100644
--- a/src/qt/transactiondesc.cpp
+++ b/src/qt/transactiondesc.cpp
@@ -1,4 +1,4 @@
-// Copyright (c) 2011-2021 The Bitcoin Core developers
+// Copyright (c) 2011-2022 The Bitcoin Core developers
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
@@ -34,18 +34,16 @@ using wallet::isminetype;
 
 QString TransactionDesc::FormatTxStatus(const interfaces::WalletTx& wtx, const interfaces::WalletTxStatus& status, bool inMempool, int numBlocks)
 {
-    {
-        int nDepth = status.depth_in_main_chain;
-        if (nDepth < 0) {
-            return tr("conflicted with a transaction with %1 confirmations").arg(-nDepth);
-        } else if (nDepth == 0) {
-            const QString abandoned{status.is_abandoned ? QLatin1String(", ") + tr("abandoned") : QString()};
-            return tr("0/unconfirmed, %1").arg(inMempool ? tr("in memory pool") : tr("not in memory pool")) + abandoned;
-        } else if (nDepth < 6) {
-            return tr("%1/unconfirmed").arg(nDepth);
-        } else {
-            return tr("%1 confirmations").arg(nDepth);
-        }
+    int nDepth = status.depth_in_main_chain;
+    if (nDepth < 0) {
+        return tr("conflicted with a transaction with %1 confirmations").arg(-nDepth);
+    } else if (nDepth == 0) {
+        const QString abandoned{status.is_abandoned ? QLatin1String(", ") + tr("abandoned") : QString()};
+        return tr("0/unconfirmed, %1").arg(inMempool ? tr("in memory pool") : tr("not in memory pool")) + abandoned;
+    } else if (nDepth < 6) {
+        return tr("%1/unconfirmed").arg(nDepth);
+    } else {
+        return tr("%1 confirmations").arg(nDepth);
     }
 }
 
diff --git a/src/qt/transactionrecord.h b/src/qt/transactionrecord.h
index dd34656d5f..b2912f17c9 100644
--- a/src/qt/transactionrecord.h
+++ b/src/qt/transactionrecord.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011-2021 The Bitcoin Core developers
+// Copyright (c) 2011-2022 The Bitcoin Core developers
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
@@ -20,13 +20,7 @@ struct WalletTxStatus;
 
 /** UI model for transaction status. The transaction status is the part of a transaction that will change over time.
  */
-class TransactionStatus
-{
-public:
-    TransactionStatus() : countsForBalance(false), sortKey(""),
-                          matures_in(0), status(Unconfirmed), depth(0), open_for(0)
-    { }
-
+struct TransactionStatus {
     enum Status {
         Confirmed,          /**< Have 6 or more confirmations (normal tx) or fully mature (mined tx) **/
         /// Normal (sent/received) transactions
@@ -40,22 +34,19 @@ public:
     };
 
     /// Transaction counts towards available balance
-    bool countsForBalance;
+    bool countsForBalance{false};
     /// Sorting key based on status
-    std::string sortKey;
+    std::string sortKey{};
 
     /** @name Generated (mined) transactions
        @{*/
-    int matures_in;
+    int matures_in{0};
     /**@}*/
 
     /** @name Reported status
        @{*/
-    Status status;
-    qint64 depth;
-    qint64 open_for; /**< Timestamp if status==OpenUntilDate, otherwise number
-                      of additional blocks that need to be mined before
-                      finalization */
+    Status status{Unconfirmed};
+    qint64 depth{0};
     /**@}*/
 
     /** Current block hash (to know whether cached status is still valid) */

@hebasto hebasto changed the title 24067 follow-ups PR24067 follow-ups Jan 26, 2022
sidhujag pushed a commit to syscoin/syscoin that referenced this issue Apr 18, 2022
…` and ` TransactionStatus`

343f83d qt, refactor: Use member initializers in TransactionStatus (w0xlt)
66d58ad qt, refactor: remove unused field `qint64 TransactionStatus::open_for` (w0xlt)
ad6aded qt, refactor: remove unused parameters in `TransactionDesc::FormatTxStatus()` (w0xlt)
045f8d0 scripted-diff: rename nDepth -> depth (w0xlt)
b1bc143 qt, refactor: remove redundant scope in `TransactionDesc::FormatTxStatus()` (w0xlt)

Pull request description:

  This PR implements the changes suggested in bitcoin-core/gui#538 (comment) .

  . remove redundant scope, rename `nDepth` -> `depth`, remove unused parameters and add translator comments in `TransactionDesc::FormatTxStatus()`
  .  Use member initializers and remove unused field `qint64 TransactionStatus::open_for` in `TransactionStatus`.

  Closes bitcoin-core/gui#538

ACKs for top commit:
  hebasto:
    ACK 343f83d, I have reviewed the code and it looks OK, I agree it can be merged.
  jarolrod:
    Code Review ACK bitcoin-core/gui@343f83d

Tree-SHA512: cc7333d85b7eb731aa8cdd2d8dfc707341532c93e1b5e3858e8341446cf055ba055b601f9662e8d4602726b1bedf13149c46256a60a0ce1a562f94c9986d945a
@bitcoin-core bitcoin-core locked and limited conversation to collaborators Apr 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants