Repository: bitcoin/bitcoin
File: src/mapport.cpp (C++)

Code before:

static CThreadInterrupt g_mapport_interrupt;
static std::thread g_mapport_thread;

using namespace std::chrono_literals;
static constexpr auto PORT_MAPPING_REANNOUNCE_PERIOD{20min};
static constexpr auto PORT_MAPPING_RETRY_PERIOD{5min};

static void ProcessPCP()
{
    // The same nonce is used for all mappings, this is allowed by the spec, and simplifies keeping track of them.
    PCPMappingNonce pcp_nonce;
    GetRandBytes(pcp_nonce);

    bool ret = false;
    bool no_resources = false;
    const uint16_t private_port = GetListenPort();
    // Multiply the reannounce period by two, as we'll try to renew approximately halfway.
    const uint32_t requested_lifetime = std::chrono::seconds(PORT_MAPPING_REANNOUNCE_PERIOD * 2).count();
    uint32_t actual_lifetime = 0;
    std::chrono::milliseconds sleep_time;

    // Local functor to handle result from PCP/NATPMP mapping.
    auto handle_mapping = [&](std::variant<MappingResult, MappingError> &res) -> void {
        if (MappingResult* mapping = std::get_if<MappingResult>(&res)) {
            LogPrintLevel(BCLog::NET, BCLog::Level::Info, "portmap: Added mapping %s\n", mapping->ToString());
            AddLocal(mapping->external, LOCAL_MAPPED);
            ret = true;
            actual_lifetime = std::min(actual_lifetime, mapping->lifetime);
        } else if (MappingError *err = std::get_if<MappingError>(&res)) {
            // Detailed error will already have been logged internally in respective Portmap function.
            if (*err == MappingError::NO_RESOURCES) {
                no_resources = true;
            }
        }
    };

    do {
        actual_lifetime = requested_lifetime;
        no_resources = false; // Set to true if there was any "no resources" error.
        ret = false; // Set to true if any mapping succeeds.

        // IPv4
        std::optional<CNetAddr> gateway4 = QueryDefaultGateway(NET_IPV4);
        if (!gateway4) {
            LogDebug(BCLog::NET, "portmap: Could not determine IPv4 default gateway\n");
        } else {
            LogDebug(BCLog::NET, "portmap: gateway [IPv4]: %s\n", gateway4->ToStringAddr());

            // Open a port mapping on whatever local address we have toward the gateway.
            struct in_addr inaddr_any;

Review comment (suggestion):
Here and elsewhere these messages had a category before and now they are category-less. When a message has a category its log is prefixed by e.g. `[net:info]` which makes it easier to filter/grep all "net" messages and to know which module a message comes from.

Would be nice to have the ability to log info/warning/error messages with category.

Code after:

static CThreadInterrupt g_mapport_interrupt;
static std::thread g_mapport_thread;

using namespace std::chrono_literals;
static constexpr auto PORT_MAPPING_REANNOUNCE_PERIOD{20min};
static constexpr auto PORT_MAPPING_RETRY_PERIOD{5min};

static void ProcessPCP()
{
    // The same nonce is used for all mappings, this is allowed by the spec, and simplifies keeping track of them.
    PCPMappingNonce pcp_nonce;
    GetRandBytes(pcp_nonce);

    bool ret = false;
    bool no_resources = false;
    const uint16_t private_port = GetListenPort();
    // Multiply the reannounce period by two, as we'll try to renew approximately halfway.
    const uint32_t requested_lifetime = std::chrono::seconds(PORT_MAPPING_REANNOUNCE_PERIOD * 2).count();
    uint32_t actual_lifetime = 0;
    std::chrono::milliseconds sleep_time;

    // Local functor to handle result from PCP/NATPMP mapping.
    auto handle_mapping = [&](std::variant<MappingResult, MappingError> &res) -> void {
        if (MappingResult* mapping = std::get_if<MappingResult>(&res)) {
            LogInfo("portmap: Added mapping %s", mapping->ToString());
            AddLocal(mapping->external, LOCAL_MAPPED);
            ret = true;
            actual_lifetime = std::min(actual_lifetime, mapping->lifetime);
        } else if (MappingError *err = std::get_if<MappingError>(&res)) {
            // Detailed error will already have been logged internally in respective Portmap function.
            if (*err == MappingError::NO_RESOURCES) {
                no_resources = true;
            }
        }
    };

    do {
        actual_lifetime = requested_lifetime;
        no_resources = false; // Set to true if there was any "no resources" error.
        ret = false; // Set to true if any mapping succeeds.

        // IPv4
        std::optional<CNetAddr> gateway4 = QueryDefaultGateway(NET_IPV4);
        if (!gateway4) {
            LogDebug(BCLog::NET, "portmap: Could not determine IPv4 default gateway\n");
        } else {
            LogDebug(BCLog::NET, "portmap: gateway [IPv4]: %s\n", gateway4->ToStringAddr());

            // Open a port mapping on whatever local address we have toward the gateway.
            struct in_addr inaddr_any;