Remove TARGET_NUMBER_OF_PEERS config variable #3852
Conversation
Co-authored-by: Justin Traglia <[email protected]>
| A node runs a background peer discovery process, maintaining at least `TARGET_NUMBER_OF_PEERS` of various custody distributions (both `custody_size` and column assignments). The combination of advertised `custody_size` size and public node-id make this readily and publicly accessible. | ||
|
|
||
| `TARGET_NUMBER_OF_PEERS` should be tuned upward in the event of failed sampling. | ||
| A node runs a background peer discovery process, maintaining at least 70 peers of various custody distributions (both `custody_size` and column assignments). The combination of advertised `custody_size` size and public node-id make this readily and publicly accessible. A node may increase its peer count in the event of failed sampling. |
There was a problem hiding this comment.
I think there is really no reason to put the number at all. How about just specifying that you should have enough peers to cover all the columns?
There was a problem hiding this comment.
Okay to me to not recommend a number at all, but would like to get the thumbs up of researchers before. The purpose of this PR is only to remove the config variable, not the number itself.
There was a problem hiding this comment.
I think removing it makes sense. We should wait for @fradamt to comment.
There was a problem hiding this comment.
Removing the config variable sounds good. As for the rest, I would maybe lean towards having a paragraph somewhere which gives some indications on how to think about the number of peers in relation to sampling, rather than giving a specific number. For example in the spirit of this discussion
There was a problem hiding this comment.
However, I think it's okay to make another PR to remove this number and rephrase this paragraph. We can merge it as it is.
|
closing in favor of #3870 |
As implementor
TARGET_NUMBER_OF_PEERShas been an awkward config variable that does not align with how the client's peer count is managed and controlled by the user. I understand its strong relevance in the context of DAS but it should not translate into it becoming a network config variable IMO.The spec can recommend a safe peer count with words on the p2p section, which is what this PR switches to. Client devs are free to implement a warning log if users select a peer count that is less than this safe value.