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

backport: merge bitcoin#23235, #23104, #24770, #24830, #24464, #24757, #25202, #25217, #25292, #25614, partial bitcoin#22766 (logging backports) #6399

Open
wants to merge 14 commits into
base: develop
Choose a base branch
from

Conversation

kwvg
Copy link
Collaborator

@kwvg kwvg commented Nov 18, 2024

Additional Information

  • This pull request's primary purpose is to restore LogPrintLevels from backports in dash#6333 that were changed to LogPrints as they were backported before LogPrintLevel was backported.
  • clang-format suggestions for LogPrintLevel have to be ignored in order to prevent the linter from tripping due to a "missing newline" (build). Resolved by applying diff (source).
  • SharedLock was introduced in dash#5961 and PrintLockContention was removed in dash#6046 but the changes in the latter were not extended to the former. This has been corrected as part of this pull request.

Breaking Changes

None expected.

Checklist

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas (note: N/A)
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have made corresponding changes to the documentation
  • I have assigned this pull request to a milestone (for repository code-owners and collaborators only)

@kwvg kwvg added this to the 22.1 milestone Nov 18, 2024
@kwvg kwvg force-pushed the logports branch 2 times, most recently from bfc4d24 to e6eef52 Compare November 19, 2024 08:45
@kwvg kwvg marked this pull request as ready for review November 20, 2024 15:58
@UdjinM6
Copy link

UdjinM6 commented Nov 20, 2024

clang-format suggestions for LogPrintLevel have to be ignored in order to prevent the linter from tripping due to a "missing newline"

should apply suggestions and add /* Continued */ at the eol to make linter happy i.e.

diff --git a/src/llmq/signing_shares.cpp b/src/llmq/signing_shares.cpp
index 0e8a5f2886..f3e53442e3 100644
--- a/src/llmq/signing_shares.cpp
+++ b/src/llmq/signing_shares.cpp
@@ -1346,11 +1346,15 @@ void CSigSharesManager::Cleanup()
                     }
                 }
 
-                LogPrintLevel(BCLog::LLMQ_SIGS, BCLog::Level::Info, "CSigSharesManager::%s -- signing session timed out. signHash=%s, id=%s, msgHash=%s, sigShareCount=%d, missingMembers=%s\n", __func__,
-                              signHash.ToString(), oneSigShare.getId().ToString(), oneSigShare.getMsgHash().ToString(), count, strMissingMembers);
+                LogPrintLevel(BCLog::LLMQ_SIGS, BCLog::Level::Info, /* Continued */
+                              "CSigSharesManager::%s -- signing session timed out. signHash=%s, id=%s, msgHash=%s, "
+                              "sigShareCount=%d, missingMembers=%s\n",
+                              __func__, signHash.ToString(), oneSigShare.getId().ToString(),
+                              oneSigShare.getMsgHash().ToString(), count, strMissingMembers);
             } else {
-                LogPrintLevel(BCLog::LLMQ_SIGS, BCLog::Level::Info, "CSigSharesManager::%s -- signing session timed out. signHash=%s, sigShareCount=%d\n", __func__,
-                              signHash.ToString(), count);
+                LogPrintLevel(BCLog::LLMQ_SIGS, BCLog::Level::Info, /* Continued */
+                              "CSigSharesManager::%s -- signing session timed out. signHash=%s, sigShareCount=%d\n",
+                              __func__, signHash.ToString(), count);
             }
             RemoveSigSharesForSession(signHash);
         }

src/sync.cpp Outdated Show resolved Hide resolved
Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

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

utACK f5231c1

doc/developer-notes.md Show resolved Hide resolved
@@ -79,6 +79,7 @@
#include <fstream>
#include <map>
#include <memory>
#include <optional>
Copy link
Member

Choose a reason for hiding this comment

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

22766; don't understand why it's partial. Missing changes, appear to be able to be done

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Will correct excluded commit in description, b8c069b is included, c5d7e34 isn't. The latter has been excluded as flags like -legacy and anything -signet*, which are affected by the scripted-diff, haven't been implemented yet.

Copy link
Collaborator

@knst knst left a comment

Choose a reason for hiding this comment

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

LGTM 1621696

@@ -3763,19 +3763,19 @@ void PeerManagerImpl::ProcessMessage(
// from switching announcement protocols after the connection is up.
if (msg_type == NetMsgType::SENDTXRCNCL) {
if (!m_txreconciliation) {
LogPrint(BCLog::NET, "sendtxrcncl from peer=%d ignored, as our node does not have txreconciliation enabled\n", pfrom.GetId());
LogPrintLevel(BCLog::NET, BCLog::Level::Debug, "sendtxrcncl from peer=%d ignored, as our node does not have txreconciliation enabled\n", pfrom.GetId());
Copy link
Collaborator

Choose a reason for hiding this comment

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

LogPrintLevel: nit: I think it's useful to specify in a commit message which exactly prior backports

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants