-
Notifications
You must be signed in to change notification settings - Fork 37.7k
doc: update BroadcastTransaction
comment
#29308
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
doc: update BroadcastTransaction
comment
#29308
Conversation
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code CoverageFor detailed information about the code coverage, see the test coverage report. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. |
BroadcastTransaction
commentBroadcastTransaction
comment
2272c3a
to
cca20d9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK cca20d9
I was looking at the same comment and was thinking it needs an update. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm ACK cca20d9
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK cca20d9
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK cca20d9
cca20d9
to
437e8eb
Compare
BroadcastTransaction
commentBroadcastTransaction
comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 437e8eb
Following the discussion in the PR so far, I agree that it is more logical and maintainable not to detail which function calls (or can call) a particular function since this information is easily accessible through other methods (e.g. by using an IDE)
That's why I think it's best to remove the comment.
BroadcastTransaction is also called by submitpackage RPC. It's not maintainable to list all the callers of a function.
437e8eb
to
31cce4a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 31cce4a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 31cce4a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 31cce4a
31cce4a doc: update `BroadcastTransaction` comment (ismaelsadeeq) Pull request description: `BroadcastTransaction` is also called by `submitpackage` RPC. All transactions that are accepted into the mempool post package processing are broadcasted to peers individually here https://github.com/bitcoin/bitcoin/blob/ea4ddd8652d9dd1e7698e2a6f84c606cf24a2e3e/src/rpc/mempool.cpp#L926 It's not maintainable to list all the callers of a function. ACKs for top commit: stickies-v: ACK 31cce4a kristapsk: ACK 31cce4a naumenkogs: ACK 31cce4a Tree-SHA512: 8aea92c53c1911a0ac36fe9e3a24d37d83e7d9b40a16f0832bfa7a719328697621e3f94a5dc80d1840e7ae705e0c3aab7a3df7064986e1e53a4a4114adf078a8
31cce4a doc: update `BroadcastTransaction` comment (ismaelsadeeq) Pull request description: `BroadcastTransaction` is also called by `submitpackage` RPC. All transactions that are accepted into the mempool post package processing are broadcasted to peers individually here https://github.com/bitcoin/bitcoin/blob/ea4ddd8652d9dd1e7698e2a6f84c606cf24a2e3e/src/rpc/mempool.cpp#L926 It's not maintainable to list all the callers of a function. ACKs for top commit: stickies-v: ACK 31cce4a kristapsk: ACK 31cce4a naumenkogs: ACK 31cce4a Tree-SHA512: 8aea92c53c1911a0ac36fe9e3a24d37d83e7d9b40a16f0832bfa7a719328697621e3f94a5dc80d1840e7ae705e0c3aab7a3df7064986e1e53a4a4114adf078a8
31cce4a doc: update `BroadcastTransaction` comment (ismaelsadeeq) Pull request description: `BroadcastTransaction` is also called by `submitpackage` RPC. All transactions that are accepted into the mempool post package processing are broadcasted to peers individually here https://github.com/bitcoin/bitcoin/blob/ea4ddd8652d9dd1e7698e2a6f84c606cf24a2e3e/src/rpc/mempool.cpp#L926 It's not maintainable to list all the callers of a function. ACKs for top commit: stickies-v: ACK 31cce4a kristapsk: ACK 31cce4a naumenkogs: ACK 31cce4a Tree-SHA512: 8aea92c53c1911a0ac36fe9e3a24d37d83e7d9b40a16f0832bfa7a719328697621e3f94a5dc80d1840e7ae705e0c3aab7a3df7064986e1e53a4a4114adf078a8
31cce4a doc: update `BroadcastTransaction` comment (ismaelsadeeq) Pull request description: `BroadcastTransaction` is also called by `submitpackage` RPC. All transactions that are accepted into the mempool post package processing are broadcasted to peers individually here https://github.com/bitcoin/bitcoin/blob/ea4ddd8652d9dd1e7698e2a6f84c606cf24a2e3e/src/rpc/mempool.cpp#L926 It's not maintainable to list all the callers of a function. ACKs for top commit: stickies-v: ACK 31cce4a kristapsk: ACK 31cce4a naumenkogs: ACK 31cce4a Tree-SHA512: 8aea92c53c1911a0ac36fe9e3a24d37d83e7d9b40a16f0832bfa7a719328697621e3f94a5dc80d1840e7ae705e0c3aab7a3df7064986e1e53a4a4114adf078a8
8bf1d06 Merge bitcoin#29308: doc: update `BroadcastTransaction` comment (glozow) 2a77808 Merge bitcoin-core/gui#789: Avoid non-self-contained Windows header (Hennadii Stepanov) da371b8 Merge bitcoin#28870: depends: Include `config.guess` and `config.sub` into `meta_depends` (fanquake) 2e41562 Merge bitcoin#29219: fuzz: Improve fuzzing stability for ellswift_roundtrip harness (fanquake) b091329 Merge bitcoin#29211: fuzz: fix `connman` initialization (Ava Chow) df42d41 Merge bitcoin#29200: net: create I2P sessions using both ECIES-X25519 and ElGamal encryption (fanquake) 4cdd1a8 Merge bitcoin#29172: fuzz: set `nMaxOutboundLimit` in connman target (fanquake) 97012ea Merge bitcoin#28962: doc: Rework guix docs after 1.4 release (fanquake) c70ff5d Merge bitcoin#28844: contrib: drop GCC MAX_VERSION to 4.3.0 in symbol-check (fanquake) e6f19e7 Merge bitcoin#29068: test: Actually fail when a python unit test fails (fanquake) 75e0334 Merge bitcoin#28989: test: Fix test by checking the actual exception instance (Andrew Chow) 8cd85d3 Merge bitcoin#28852: script, assumeutxo: Enhance validations in utxo_snapshot.sh (Ryan Ofsky) fd2e88d Merge bitcoin#26077: guix: switch from `guix environment` to `guix shell` (fanquake) 02741a7 Merge bitcoin#28913: coins: make sure PoolAllocator uses the correct alignment (fanquake) dfd53da Merge bitcoin#28902: doc: Simplify guix install doc, after 1.4 release (fanquake) Pull request description: ## Issue being fixed or feature implemented Batch of trivial backports ## What was done? See commits ## How Has This Been Tested? built locally; large combined merge passed tests locally ## Breaking Changes Should be none ## 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 - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: UdjinM6: utACK 8bf1d06 knst: utACK 8bf1d06 Tree-SHA512: 506273e5a188f9ca74edf656e3cd338992192e6e97f68c89fc43e34be20fb7f211b48e4dfa8693727839a7920da8284509413c722f55774a428939c296dad517
BroadcastTransaction
is also called bysubmitpackage
RPC.All transactions that are accepted into the mempool post package processing are broadcasted to peers individually here
bitcoin/src/rpc/mempool.cpp
Line 926 in ea4ddd8
It's not maintainable to list all the callers of a function.