Skip to content
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

No validation of minimum amount of overlap/nodes (Version 2.4.0-b3) #5303

Open
shortthefomo opened this issue Feb 20, 2025 · 5 comments
Open
Assignees

Comments

@shortthefomo
Copy link

shortthefomo commented Feb 20, 2025

Issue Description

The recently merged change in #5112 moves to a intersection threshold however it does not guarantee a minimum number of nodes are met.

Steps to Reproduce

Create 3 UNL lists with only 1 node that is common (as an extreme example).

Expected Result

I would expect there to be some behavior prior to a low number of nodes in the overlap. Giving node operators and list maintainers time to adjust.

A solution here could also be we want to maintain a minimum number of nodes are in the UNL, if that number additionally is also not met (once the intersection kicks in) the network falls back to the default case of the union on the default UNL.

@shortthefomo shortthefomo changed the title No validation of minimum amount of overlap/nodes (Version: Version 2.4.0-b3) No validation of minimum amount of overlap/nodes (Version 2.4.0-b3) Feb 20, 2025
@bthomee bthomee assigned bthomee and Bronek and unassigned bthomee Feb 20, 2025
@mov-bx-0xb800

This comment has been minimized.

@ximinez
Copy link
Collaborator

ximinez commented Feb 20, 2025

Here's the thing: If I intentionally subscribe to a UNL that only has one validator on it (not a good idea, but don't judge, maybe it's for testing), that should be a perfectly valid config. There is no one-size-fits-all minimum value.

If there is a check, it should be configurable, maybe with a default based on the size of the original UNLs (maybe some percentage of the median size, or size of the smallest?)

@shortthefomo
Copy link
Author

shortthefomo commented Feb 20, 2025

Not quite, im pointing out say each list had 30+ nodes on it but only 1 was common. The case this becomes more of an issue is as more lists are added the intersection will naturally decrease.

So example in the opposite direction of the one initially added. We have 20 lists (each with 20+ nodes on them) that are use to compose the UNL only 5 nodes across all those are now in the intersection . Sure if that's a good place to find the network in.

I don't know what is a minimum number of nodes the UNL should contain but as you suggest a configurable minimum is probably the correct thing to do.

@ximinez
Copy link
Collaborator

ximinez commented Feb 20, 2025

Not quite, im pointing out say each list had 30+ nodes on it but only 1 was common. The case this becomes more of an issue is as more lists are added the intersection will naturally decrease.

I hear what you're saying. My point is more that this is a semi-manual configuration process, especially now since the default configuration only has two UNL providers, which will still use a union. If an operator chooses to configure their node such that this situation is possible, that's their choice. We can only do so much to prevent someone from shooting themselves in the foot.

The intersection functionality was intended as a way to reduce the attack surface of the XRPL. Properly configured, it should be very unlikely to get into this kind of situation. For example, if I wanted to use 3 UNLs, I would choose 3 UNLs that were identical or almost identical, and require an intersection of 2 for a validator to count. (I'm pretty sure that's the default behavior, if I don't specify a value.)

Now imagine one of those UNL publishers gets compromised and the attacker sticks 100 validators that they control on the list. Because a validator has to be on 2 UNLs, my node will ignore them. Good. Imagine the attacker removes all but one validator from the list. Because (most of) those validators are on the other two lists, my node will basically carry on unchanged. Good.

For the overlap to be reduced significantly, either I'd have to start with badly diverged UNLs, or an attacker would have to compromise multiple of the publishers.

So example in the opposite direction of the one initially added. We have 20 lists (each with 20+ nodes on them) that are use to compose the UNL only 5 nodes across all those are now in the intersection . Sure if that's a good place to find the network in.

The default configuration will never intentionally create this situation. An individual node operator can make whatever stupid decisions they want. If they're not on a UNL, no one will care. If they are on one or more, and their node becomes unreliable because of their stupid decision, they'll probably be removed pretty quickly.

I don't know what is a minimum number of nodes the UNL should contain but as you suggest a configurable minimum is probably the correct thing to do.

On further thought, The configuration option shouldn't just be a hard value either, because I don't want to have to update it every time a publisher adds or removes a validator. Something like "minimum percentage needed of the smallest UNL" or percentage of the union.

@shortthefomo
Copy link
Author

shortthefomo commented Feb 20, 2025

For the overlap to be reduced significantly, either I'd have to start with badly diverged UNLs

My understanding of this change as more lists are adopted is that the overlap and worry of the high percentage +80% becomes less worried about (would assume only the dUNLs would need to keep making sure the minimum overlap exists incase reversion to two list setup is needed).

As the network adopts new lists, they naturally also will and should diverge more, the other published lists.

Thanks just glad the issues i've put forward have been considered.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants