Skip to content

doc: better document NetEventsInterface and the deletion of "CNode"s#32278

Open
vasild wants to merge 1 commit into
bitcoin:masterfrom
vasild:doc_NetEventsInterface
Open

doc: better document NetEventsInterface and the deletion of "CNode"s#32278
vasild wants to merge 1 commit into
bitcoin:masterfrom
vasild:doc_NetEventsInterface

Conversation

@vasild

@vasild vasild commented Apr 15, 2025

Copy link
Copy Markdown
Contributor

Document the requirements around the NetEventsInterface's methods and the lifetime of CNode objects in CConnman::m_nodes.

@DrahtBot

DrahtBot commented Apr 15, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32278.

Reviews

See the guideline for information on the review process.

Type Reviewers
Stale ACK w0xlt

If your review is incorrectly listed, please copy-paste <!--meta-tag:bot-skip--> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #35600 (net: prevent duplication manual connections (take 2) by willcl-ark)

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.

LLM Linter (✨ experimental)

Possible typos and grammar issues:

  • no-referenced CNode -> unreferenced CNode [“no-referenced” is not valid English and obscures the intended meaning]

2026-06-22 13:37:05

@DrahtBot DrahtBot added the Docs label Apr 15, 2025
@vasild vasild mentioned this pull request Apr 15, 2025
Open
@vasild vasild force-pushed the doc_NetEventsInterface branch 2 times, most recently from 0f26ac3 to e44e669 Compare April 16, 2025 04:15
@vasild

vasild commented Apr 16, 2025

Copy link
Copy Markdown
Contributor Author

1091faf8ff...0f26ac35e2 and 0f26ac35e2...e44e669378: further elaborate how we safely access CNodes in m_nodes and how we avoid CNode destruction while it is being referenced.

@DrahtBot DrahtBot mentioned this pull request Apr 16, 2025
Draft
w0xlt
w0xlt reviewed Apr 17, 2025
View reviewed changes

@w0xlt w0xlt left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ACK e44e669

Clear improvement in documentation.

@DrahtBot DrahtBot mentioned this pull request May 1, 2025
Merged
@DrahtBot DrahtBot removed the CI failed label May 2, 2025
@DrahtBot DrahtBot mentioned this pull request Jul 14, 2025
Closed
laanwj
laanwj reviewed Oct 23, 2025
View reviewed changes
Comment thread src/net.h Outdated
ajtowns
ajtowns reviewed Oct 29, 2025
View reviewed changes
Comment thread src/net.h Outdated
Comment thread src/net.h Outdated
ajtowns
ajtowns reviewed Oct 29, 2025
View reviewed changes
Comment thread src/net.h
@vasild vasild force-pushed the doc_NetEventsInterface branch from e44e669 to 110548c Compare November 13, 2025 15:42
@vasild

vasild commented Nov 13, 2025

Copy link
Copy Markdown
Contributor Author

e44e669378...110548c9ea: take suggestions

@DrahtBot DrahtBot mentioned this pull request Dec 17, 2025
Merged
@DrahtBot DrahtBot mentioned this pull request Dec 31, 2025
Merged
@sedited

sedited commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

@vasild can you rebase this?

@vasild @ajtowns
doc: better document NetEventsInterface and the deletion of "CNode"s
5647c18 
Document the requirements around the `NetEventsInterface`'s methods and
the lifetime of `CNode` objects in `CConnman::m_nodes`.

Co-authored-by: Anthony Towns <aj@erisian.com.au>
@vasild vasild force-pushed the doc_NetEventsInterface branch from 110548c to 5647c18 Compare June 22, 2026 13:36
@vasild

vasild commented Jun 22, 2026

Copy link
Copy Markdown
Contributor Author

110548c9eabc0bdd1341e4aafe9fe3f9119ed7be...5647c188138ae03cdb15db7a99b7e6b160bb9ba0: rebase due to conflicts, thanks for the ping!

@DrahtBot DrahtBot mentioned this pull request Jun 25, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@laanwj laanwj laanwj left review comments
@theuni theuni Awaiting requested review from theuni
@sipa sipa Awaiting requested review from sipa
+2 more reviewers
@ajtowns ajtowns ajtowns left review comments
@w0xlt w0xlt w0xlt left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Docs

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@vasild @DrahtBot @sedited @laanwj @ajtowns @w0xlt