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

Network Watch tool #444

Closed
wants to merge 2 commits into from
Closed

Conversation

luke-jr
Copy link
Member

@luke-jr luke-jr commented Oct 3, 2021

Now that the crash bug (bitcoinknots/bitcoin#4) has been tracked down, this seems ready for re-opening for Core.

(Originally bitcoin/bitcoin#9849)

@hebasto hebasto added the Feature label Oct 3, 2021
@hebasto hebasto changed the title Qt: Network Watch tool Network Watch tool Oct 3, 2021
@jarolrod
Copy link
Member

jarolrod commented Oct 3, 2021

Concept ACK

In general, I'm in favor of not having tons of pop-out windows. May be worth exploring this as a new tab.

If possible, can you include screenshots of this in action in the OP?

@luke-jr
Copy link
Member Author

luke-jr commented Oct 3, 2021

Not sure how to add inline images on GitHub, but there's a sceenshot on the wiki: https://en.bitcoin.it/wiki/File:Bitcoinknots-netwatch.png

@luke-jr
Copy link
Member Author

luke-jr commented Oct 3, 2021

As for "pop out windows", I think that's the correct UX for features like this. Having a tab in the main window just doesn't seem to fit well.

Maybe a new debug-window-alike for statistics?

Copy link
Member

@jarolrod jarolrod left a comment

Choose a reason for hiding this comment

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

Have not yet reviewed the code, but:

Commit 47711fd must be broken off and opened in the core repo if it is needed. Then this can be based on the PR that would be open. The change to to validationinterface cannot be done in this repo.

Additionally, the commit history is not clean. The last 4 commits could be squashed onto ae6804e

@DrahtBot
Copy link
Contributor

DrahtBot commented Jan 24, 2022

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.

Type Reviewers
Concept NACK rebroad
Concept ACK jarolrod, jonatack

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #553 (Change address / amount error background by w0xlt)
  • #537 (Point out position of invalid characters in Bech32 addresses by luke-jr)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

luke-jr added 2 commits May 1, 2022 19:16
Simple realtime log of p2p network activity (blocks and transactions only)

- Doesn't begin logging until opened; limited to 0x400 entries (outputs)
- Automatically scrolls if left at the bottom of the log; maintains position if left elsewhere
- Memory-efficient circular buffer; CTransaction references become weak after they're 0x200 entries back in the log
- Search function that selects all matching log entries, including ongoing
@DrahtBot
Copy link
Contributor

🐙 This pull request conflicts with the target branch and needs rebase.

Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a "draft".

@rebroad
Copy link
Contributor

rebroad commented Sep 3, 2022

concept NACK - at least, so far, I'm struggling to understand the use case for this. I'd have thought something more command line would make sense, as surely there'd be so much data that it would scroll by too quickly to be useful.

@jonatack
Copy link
Member

jonatack commented Sep 5, 2022

Concept ACK if I'm understanding from the earlier PR. Suggest describing what it does and the use case (what and why) in the pull description.

@DrahtBot
Copy link
Contributor

DrahtBot commented Dec 4, 2022

There hasn't been much activity lately and the patch still needs rebase. What is the status here?

  • Is it still relevant? ➡️ Please solve the conflicts to make it ready for review and to ensure the CI passes.
  • Is it no longer relevant? ➡️ Please close.
  • Did the author lose interest or time to work on this? ➡️ Please close it and mark it 'Up for grabs' with the label, so that it can be picked up in the future.

2 similar comments
@DrahtBot
Copy link
Contributor

DrahtBot commented Mar 4, 2023

There hasn't been much activity lately and the patch still needs rebase. What is the status here?

  • Is it still relevant? ➡️ Please solve the conflicts to make it ready for review and to ensure the CI passes.
  • Is it no longer relevant? ➡️ Please close.
  • Did the author lose interest or time to work on this? ➡️ Please close it and mark it 'Up for grabs' with the label, so that it can be picked up in the future.

@DrahtBot
Copy link
Contributor

DrahtBot commented Jun 2, 2023

There hasn't been much activity lately and the patch still needs rebase. What is the status here?

  • Is it still relevant? ➡️ Please solve the conflicts to make it ready for review and to ensure the CI passes.
  • Is it no longer relevant? ➡️ Please close.
  • Did the author lose interest or time to work on this? ➡️ Please close it and mark it 'Up for grabs' with the label, so that it can be picked up in the future.

@hebasto
Copy link
Member

hebasto commented Jun 4, 2023

Closing this due to lack of activity. Feel free to reopen.

@hebasto hebasto closed this Jun 4, 2023
@bitcoin-core bitcoin-core locked and limited conversation to collaborators Jun 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants