Skip to content

Conversation

@PastaPastaPasta
Copy link
Member

Issue being fixed or feature implemented

Batch of trivial backports

What was done?

Trivial backports

How Has This Been Tested?

Built; haven't ran tests

Breaking Changes

Didn't see any

Checklist:

Go over all the following points, and put an x in all the boxes that apply.

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • 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)

jonasschnelli and others added 6 commits June 4, 2024 12:50
…data layer violation

b3e9bca qt, refactor: Drop no longer used PeerTableModel::getNodeStats function (Hennadii Stepanov)
49c6040 qt: Use PeerTableModel::StatsRole (Hennadii Stepanov)
35007ed qt: Add PeerTableModel::StatsRole (Hennadii Stepanov)

Pull request description:

  This PR allows to access to the `CNodeCombinedStats` instance directly from any view object.

  The `PeerTableModel::getNodeStats` member function removed as a kind of layer violation.

  No behavior changes.

  Also other pulls (bugfixes) are based on this one: #18 and dashpay#164.

ACKs for top commit:
  jonatack:
    Tested re-ACK b3e9bca per `git range-diff ae8f797 4c05fe0 b3e9bca`
  promag:
    Code review ACK b3e9bca.
  jonasschnelli:
    utACK b3e9bca

Tree-SHA512: 6ba50d5dd2c0373655d491ce8b130c47d598da2db5ff4b00633f447404c7e70f8562ead53ddf166e851384d9632ff9146a770c99845c2cdd3ff7250677e4c130
c9095b7 test: remove unnecessary assignment in bdb (Bruno Garcia)

Pull request description:

  This PR removes the unnecessary assignment to page_info['entries'] on line 54 since there is another assignment for it in line 59.

  I think a lint (bitcoin#21096) would detect cases like this one.

ACKs for top commit:
  achow101:
    ACK c9095b7
  theStack:
    Code Review ACK c9095b7

Tree-SHA512: 23377077c015b04361fd416b41bf6806ad0bdd4d264be6760f0fd3bc88d694d2cd52cae250519925c5d3b3c70715772714c3863f8fa181a2eb4883204ccdbf9d
8353e8c peers-tab: bug fix right panel toggle (randymcmillan)

Pull request description:

  Initial Presentation:

  ![Screen Shot 2021-01-28 at 8 36 15 PM](https://user-images.githubusercontent.com/152159/106220159-e2a81b80-61a8-11eb-84e9-f9b44375c9a1.png)

  When node row selected - panel is presented:

  ![Screen Shot 2021-01-28 at 8 36 22 PM](https://user-images.githubusercontent.com/152159/106220185-eb98ed00-61a8-11eb-9467-6a762941902d.png)

  When network disabled - right panel is hidden:

  ![Screen Shot 2021-01-28 at 8 36 32 PM](https://user-images.githubusercontent.com/152159/106220235-0a977f00-61a9-11eb-8a10-f31e4312ed31.png)

ACKs for top commit:
  jarolrod:
    ACK 8353e8c
  jonatack:
    ACK 8353e8c tested rebased on current master. Behavior is initially a bit surprising but this would allow more columns to be added to the peers tab window. Verified that selecting more than one peer, clicking on a column header, or running `disconnectnode "" <currently-selected-peer-id>` in the console (or on the CLI with the `-server` startup option) returns the window to its full size. If this is merged, it might be nice to have an obvious way to close the details area like a clickable "close this" icon in the upper left corner of the area.
  Talkless:
    tACK 8353e8c, tested on Debian Sid. Made `bitcoind` connect to `bitcoin-qt` with the PR changes, and after I quit the `bitcoind` instance, right panel do disappear, compared to the previous commit where it didn't.

Tree-SHA512: 8fc156f40bdd61e3ba8db333c729a2a07fd5f0fd1eed56f2fd2aa5ae5864756f8ab6fad74ae2fb0552ee7518b6d489f5800709e6c80c6f31f61fd8ce21cece5f
…nnect except for noban and manual peers

fa55159 net: Log to net debug in MaybeDiscourageAndDisconnect except for noban and manual peers (MarcoFalke)

Pull request description:

  The goal is to avoid local peers (e.g. untrusted peers on the local network or inbound onion peers via a local onion proxy) filling the debug log (and thus the disk).

ACKs for top commit:
  practicalswift:
    ACK fa55159
  vasild:
    ACK fa55159

Tree-SHA512: de233bf57334580f9b91f369fafd131d71c5ae25db25b09cc8fa8cbf34c0648f083c52260a6a912238751467e3c3c5f5d2309c145710753058d44a0003f88f4f
def1e64 scripted-diff: Drop redundant QString calls (Hennadii Stepanov)

Pull request description:

  The return type of `QObject::tr` function _is_ `QString` 🐅

ACKs for top commit:
  jarolrod:
    ACK def1e64, tested on macOS 10.14.6 Qt 5.15.2

Tree-SHA512: ef405c87a30d6965f6887511d8666b6da57d258ca07833a3fa2dc9fd147d0539d33c57f7551ee13c1dd8024d6057139595c6ce5d088dd6efd7aa13db2a3eebdb
…to fix duration, improve BCLog::LogMsg()

f530202 Make unexpected time type in BCLog::LogMsg() a compile-time error (Martin Ankerl)
bddae7e Add util/types.h with ALWAYS_FALSE template (MarcoFalke)
498b323 log, timer: improve BCLog::LogMsg() (Jon Atack)
8d2f847 sync: inline lock contention logging macro to fix time duration (Jon Atack)

Pull request description:

  Follow-up to bitcoin#22736.

  The first commit addresses the issue identified and reported by Martin Ankerl in bitcoin#22736 (comment) to fix the lock contention duration reporting.

  The next three commits make improvements to the timer code in `BCLog::LogMsg()` and add `util/types.h` with an `ALWAYS_FALSE` template, that springboard from bitcoin#22736 (comment) by Marco Falke.

ACKs for top commit:
  martinus:
    re-ACK f530202. I ran a fully synced node for about a day. My node was mostly idle though so not much was going on. I [wrote a little script](https://github.com/martinus/bitcoin-stuff/blob/main/scripts/parse-debuglog-contention-single.rb) to parse the `debug.log` and summarize the output to see if anything interesting was going on, here is the result:
  theStack:
    ACK f530202

Tree-SHA512: 37d093eac5590e1b5846ab5994d0950d71e131177d1afe4a5f7fcd614270f977e0ea117e7af788e9a74ddcccab35b42ec8fa4db3a3378940d4988df7d21cdaaa
@PastaPastaPasta PastaPastaPasta added this to the 21 milestone Jun 4, 2024
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.

tests failed, reverting/dropping 22817 (50e34fb) helps

@PastaPastaPasta PastaPastaPasta force-pushed the develop-trivial-2024-06-04 branch from c9d1377 to d22d429 Compare June 4, 2024 20:13
@UdjinM6
Copy link

UdjinM6 commented Jun 4, 2024

Hmm... Looks like wallet_multisig_descriptor_psbt.py is broken too. Drop 22067 (2ce0367) and 23303 (c5860fd) for now maybe?

095f077 ci: Do not print `git log` for empty COMMIT_RANGE (Hennadii Stepanov)

Pull request description:

  On master (77a2f5d) a CI lint task [log](https://api.cirrus-ci.com/v1/task/4817858858319872/logs/lint.log) exceeds 20K lines.

  This PR fixes this issue.

ACKs for top commit:
  MarcoFalke:
    cr ACK 095f077

Tree-SHA512: 89180018aeccf1599cdf218924cbab12dcbae0f6674bb90e13b64e342cdd908a880b885039c23f0d1d03493e55a94fe04abf39481616ae6550c6a759f5ca9a35
@PastaPastaPasta PastaPastaPasta force-pushed the develop-trivial-2024-06-04 branch from d22d429 to a62e170 Compare June 5, 2024 02:20
@PastaPastaPasta PastaPastaPasta requested a review from UdjinM6 June 5, 2024 05:27
Copy link
Member Author

@PastaPastaPasta PastaPastaPasta left a comment

Choose a reason for hiding this comment

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

self-review utACK

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

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.

utACK a62e170

@PastaPastaPasta PastaPastaPasta merged commit d23ecf8 into dashpay:develop Jun 5, 2024
@PastaPastaPasta PastaPastaPasta deleted the develop-trivial-2024-06-04 branch June 5, 2024 13:36
PastaPastaPasta added a commit that referenced this pull request Nov 26, 2024
, bitcoin#24830, bitcoin#24464, bitcoin#24757, bitcoin#25202, bitcoin#25217, bitcoin#25292, bitcoin#25614, partial bitcoin#22766 (logging backports)

1621696 log: restore `LogPrintLevel` messages from prior backports (Kittywhiskers Van Gogh)
52a1263 merge bitcoin#25614: Severity-based logging, step 2 (Kittywhiskers Van Gogh)
21470fd merge bitcoin#25292: Add LogPrintLevel to lint-format-strings, drop LogPrint-vs-LogPrintf section in dev notes (Kittywhiskers Van Gogh)
026409e merge bitcoin#25217: update lint-logs.py to detect LogPrintLevel, mention WalletLogPrintf (Kittywhiskers Van Gogh)
b046e09 merge bitcoin#25202: Use severity-based logging for leveldb/libevent messages, reverse LogPrintLevel order (Kittywhiskers Van Gogh)
7697b73 revert dash#2794: Disable logging of libevent debug messages (Kittywhiskers Van Gogh)
ff6304f merge bitcoin#24757: add `DEBUG_LOCKCONTENTION` to `--enable-debug` and CI (Kittywhiskers Van Gogh)
88592f3 merge bitcoin#24464: Add severity level to logs (Kittywhiskers Van Gogh)
d3e837a merge bitcoin#24830: Allow -proxy="" setting values (Kittywhiskers Van Gogh)
0e01d5b partial bitcoin#22766: Clarify and disable unused ArgsManager flags (Kittywhiskers Van Gogh)
a9cfbd1 fix: don't use non-existent `PrintLockContention` in `SharedEnter` (Kittywhiskers Van Gogh)
f331cbe merge bitcoin#24770: Put lock logging behind DEBUG_LOCKCONTENTION preprocessor directive (Kittywhiskers Van Gogh)
d9cc2ea merge bitcoin#23104: Avoid breaking single log lines over multiple lines in the log file (Kittywhiskers Van Gogh)
479ae82 merge bitcoin#23235: Reduce unnecessary default logging (Kittywhiskers Van Gogh)

Pull request description:

  ## Additional Information

  * This pull request's primary purpose is to restore `LogPrintLevel`s from backports in [dash#6333](#6333) that were changed to `LogPrint`s 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](https://gitlab.com/dashpay/dash/-/jobs/8398818860#L54)).~~ Resolved by applying diff ([source](#6399 (comment))).
  * `SharedLock` was introduced in [dash#5961](#5961) and `PrintLockContention` was removed in [dash#6046](#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

  - [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
  - [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)_

Top commit has no ACKs.

Tree-SHA512: f2d0ef8ce5cb1091c714a2169e89deb33fa71ff174ce4e6147b3ad421f57a84183d2a9e76736c0b064b2cc70fb3f2e545c42b8562cf36fdce18c3fb61307c364
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