doc: Remove TODO 'exclude peers with download permission' #23128

Following from PR #23109 The TODO is no longer necessary. Removing it to prevent future confusion.

Btw, the TODO has been added by @jonasschnelli in commit 872fee3fccc8b33b9af0a401b5f85ac5504b57eb. It wasn't added by me.

Sorry for insisting, but I think a documentation improvement is a necessary step while touching these lines.

Btw, the TODO has been added by @jonasschnelli in commit 872fee3. It wasn't added by me.

Whoops, blame git blame.

As I was writing some docs,I realized that GetTime is always called, even when nMaxOutboundLimit is zero (i.e. no -maxuploadtarget is set)

Shouldn't the code look like this:

    if (nMaxOutboundLimit) {
        const auto now = GetTime<std::chrono::seconds>();
        if (nMaxOutboundCycleStartTime + MAX_UPLOAD_TIMEFRAME < now)
        {
            // timeframe expired, reset cycle
            nMaxOutboundCycleStartTime = now;
            nMaxOutboundTotalBytesSentInCycle = 0;
        }
    }

Or should we call GetMaxOutboundTarget ? There's a lock in that method. Maybe that's too expensive of a call?

@dougEfresh I'd say it's a borderline improvement. The fact that maxuploadtarget=0 currently means no limit is unknown for RecordBytesSent, and that's not necessarily bad.

If you want the function to be aware of this policy, yeah, your code will work, if you accompany it with the comment. One thing to keep in mind that nMaxOutboundCycleStartTime, as exposed to the user, would be different from other peers, because it will be never updated. Might be counter-intuitive.

So, yeah, I'm not sure it's worth it.

I think for this PR it makes sense to limit it to documentation improvement. Changing the logic can be done later (or not).

I added blocks created within past week to -maxuploadtarget help message, as I found that useful to know. This was referenced in doc/reduce-traffic.md I'm not sure what other documentation to add for clarification. Guidance appreciated

I think it would be great to integrate the comment I provided here, perhaps in a better shape and potentially in different related places of the codebase :)

<!--e57a25ab6845829454e8d69fc972939a-->

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

<!--174a7506f384e20aa4161008e828411d-->

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #23249 (util: ParseByteUnits - Parse a string with suffix unit by dougEfresh)

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.

why was this closed?

I'm going to remove the my forked repo, I can open it after I rename.

ping