scripted-diff: Modernize nLocalServices naming #30885

pull fjahr wants to merge 1 commits into bitcoin:master from fjahr:2024-09-localservices-new changing 3 files +13 −13

fjahr commented at 5:33 PM on September 12, 2024: contributor

The type of the nLocalServices variable was changed to std::atomic<ServiceFlags> in #30807 and I suggested the variable name to get updated with a scripted diff along with it. It wasn't included in the PR but I am still suggesting to do it as a follow-up since I had already prepared the commit.
DrahtBot commented at 5:33 PM on September 12, 2024: contributor


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



Code Coverage

For detailed information about the code coverage, see the test coverage report.



Reviews

See the guideline for information on the review process.

Type Reviewers

ACK furszy, jonatack, theStack, sipa, achow101

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:
- #30951 (net: option to disallow v1 connection on ipv4 and ipv6 peers by stratospher)
- #29415 (Broadcast own transactions only via short-lived Tor or I2P connections by vasild)
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.
DrahtBot added the label Refactoring on Sep 12, 2024

Type	Reviewers
ACK	furszy, jonatack, theStack, sipa, achow101

scripted-diff: Modernize nLocalServices to m_local_services

-BEGIN VERIFY SCRIPT-
sed -i 's/nLocalServices/m_local_services/g' src/net.h src/net.cpp
sed -i 's/connOptions.nLocalServices/connOptions.m_local_services/g' src/init.cpp
sed -i 's/nLocalServices/g_local_services/g' src/init.cpp
-END VERIFY SCRIPT-

33381ea530

in src/init.cpp:843 in 0e5cfd253e outdated
```
 839 | @@ -840,7 +840,7 @@ namespace { // Variables internal to initialization process only
 840 |  
 841 |  int nMaxConnections;
 842 |  int available_fds;
 843 | -ServiceFlags nLocalServices = ServiceFlags(NODE_NETWORK_LIMITED | NODE_WITNESS);
 844 | +ServiceFlags local_services = ServiceFlags(NODE_NETWORK_LIMITED | NODE_WITNESS);
```
maflcko commented at 6:00 PM on September 12, 2024:

globals should be g_, no? See also g_enabled_filter_types two lines below.

fjahr commented at 8:20 PM on September 12, 2024:

Hm, ok, I interpreted the comment on the namespace above as that we ignore that here. But I missed the variable below and I the others are just older. Changed.
fjahr force-pushed on Sep 12, 2024
DrahtBot added the label CI failed on Sep 12, 2024

in src/net.cpp:2948 in 33381ea530

2944 | @@ -2945,7 +2945,7 @@ void CConnman::OpenNetworkConnection(const CAddress& addrConnect, bool fCountFai
2945 |          return;
2946 |      pnode->grantOutbound = std::move(grant_outbound);
2947 |  
2948 | -    m_msgproc->InitializeNode(*pnode, nLocalServices);
2949 | +    m_msgproc->InitializeNode(*pnode, m_local_services);

furszy commented at 8:59 PM on September 12, 2024:

tiny nit: I know this is an scripted-diff but.. could call GetLocalServices() here.

furszy commented at 9:00 PM on September 12, 2024: member

utACK 33381ea530ad79ac1e04c37f5707e93d3e0509ca
jonatack commented at 9:06 PM on September 12, 2024: member

ACK 33381ea530ad79ac1e04c37f5707e93d3e0509ca

Following the last push, suggest renaming the pull title to scripted-diff: Modernize nLocalServices naming (edit: and also the commit name)
fjahr renamed this:
~~scripted-diff: Modernize nLocalServices to m_local_services~~
scripted-diff: Modernize nLocalServices naming
on Sep 12, 2024
fjahr commented at 9:16 PM on September 12, 2024: contributor

Following the last push, suggest renaming the pull title to scripted-diff: Modernize nLocalServices naming (edit: and also the commit name)

done
mzumsande commented at 9:48 PM on September 12, 2024: contributor

For the record, I don't like refactor PRs that just rename things to a new convention. It's fine for me if the code is touched anyway, but opening a PR just for that? We probably have hundreds of variables that could be modernized the same way.
fjahr commented at 9:51 PM on September 12, 2024: contributor

For the record, I don't like refactor PRs that just rename things to a new convention. It's fine for me if the code is touched anyway, but opening a PR just for that? We probably have hundreds of variables that could be modernized the same way.

Sure, I agree and I had not opened it if I had not already written the commit and had not been encouraged to open it as a follow-up even though the PR where the code was touched was merged without it, see #30807 (comment).
DrahtBot removed the label CI failed on Sep 13, 2024
theStack approved
theStack commented at 10:29 AM on September 17, 2024: contributor

ACK 33381ea530ad79ac1e04c37f5707e93d3e0509ca
sipa commented at 10:11 PM on September 30, 2024: member

utACK 33381ea530ad79ac1e04c37f5707e93d3e0509ca
bitcoin deleted a comment on Oct 1, 2024
achow101 commented at 6:49 PM on October 8, 2024: member

ACK 33381ea530ad79ac1e04c37f5707e93d3e0509ca
achow101 merged this on Oct 9, 2024
achow101 closed this on Oct 9, 2024
TheCharlatan referenced this in commit 8bb47d4c2c on Nov 2, 2024
bug-castercv502 referenced this in commit 403bebd591 on Sep 28, 2025
bitcoin locked this on Oct 9, 2025