les: les/4 protocol extensions #21676

zsfelfoldi · 2020-10-08T02:37:44Z

This WIP PR implements

lespay UDP communication via the new discv5 Talk request
a simple DDoS protection mechanism (see Use utils.Limiter for discv5 devp2p#162 )
the lespay capacity query (available capacity vs. purchased tokens curve)
les/4 protocol changes:
- serve lespay messages through LES TCP connection
- allow higher capacity request for priority clients
- allow inactive (zero capacity) LES connections for a short time; this is necessary for price negotiation and token purchase if the client cannot use UDP
- replace the ReqID and BufferValue message fields with a future-proof "metainfo channel" that can contain any number of mutually understood meta fields per message
- add individual request cost and token balance feedback fields to the new metainfo structure (optional)

TO DO:

limit frequency of capacity requests

fjl · 2020-10-14T14:20:18Z

The discv5 implementation is now merged on master, you can rebase on that.

rjl493456442 · 2020-11-20T08:04:12Z

Zsolt! My first suggestion is to separate this PR into a few small ones. I would really like to have the first PR for the UDP limitation!

And the second suggestion is about the UDP limitation. I would say the double selection is a good idea: one for DoS protection, one for DDoS protection. However now it looks like we are making another wheel as the TCP limitation. In les tcp protocol, we limit the request serving by the time cost, response size. And imho it's already really complicated. I really don't want to make the udp stuff that much complicated.

As far as I see, the udp protocol should be much simpler? Can we make the UDP handler kind of not cpu-bounded?
For example, only support some simple query, simple acitivities? So that we can hold the assumption that the serving cost is basically equal? In this way we can delete all the relCost stuff, all the costFilter, all the ... stuff.

rjl493456442 · 2020-11-20T08:07:19Z

If I understand correctly, the main purpose of using UDP is: we still want to support some communications between the servers and clients before establishing the TCP connection. (e.g. query the price, buy the tokens). All the complicated communications should be put into the TCP protocol, right? It's unnecessary to put all of them into the UDP.

rjl493456442 · 2020-11-20T08:09:03Z

And even more, the functionalities in the UDP level should be limited(imho, query price, buy tokens, query the average performance statistics, like the average latency should be enough). We don't want to expose too much information via the UDP, especially some expensive query APIs.

rjl493456442 · 2020-11-23T06:38:05Z

les/utils/limiter.go

+	addressSelect, valueSelect                        *WeightedRandomSelect
+	maxValue                                          float64
+	maxPriorWeight, totalPriorWeight, totalPriorLimit uint64
+	lastValueSelected                                 bool


Perhaps selectedByValue is a better name?

priorWeight is confusing for me. PriorWeight is a rough estimate of the serving cost of the request., perhaps we can pick a better name?

And also for totalPriorWeight, perhaps we can use cumulativeXXX. Because the requests are accumulated

I dropped the prior weight vs. variable cost logic in order to make things simpler.

rjl493456442 · 2020-11-23T06:54:22Z

les/utils/limiter.go

+		panic("added node queue is already in an address group")
+	}
+	l := len(ag.nodes)
+	if l == 1 {


I don't understand why we need to specialize the single element set?

If there is a single element in the selection set, we can still build the set for it

Done. The original version was meant to be a performance optimization but I tried and it does not make a big difference (less than 10%).

rjl493456442 · 2020-11-23T06:57:16Z

les/utils/limiter.go

+	nq.groupIndex = -1
+	ag.nodes = ag.nodes[:l]
+	ag.sumFlatWeight -= nq.flatWeight
+	if l >= 1 {


Also If we can remove the specialization for the single element set, it will be simplified a lot

rjl493456442 · 2020-11-23T07:00:48Z

les/utils/limiter.go

+// selectionWeights calculates the selection weights of a node for both the address and
+// the value selector. The weight depends on the relative cost of the previous processed
+// request.
+func selectionWeights(relCost, value float64) (flatWeight, valueWeight uint64) {


As I commented, the flatweight is derived by some means. It's not fair anymore...

I am not sure if I understand you correctly but the flat weight is divided if there are multiple nodes requesting from the same IP address. This protects against Sybil attacks using many node keys.

rjl493456442 · 2020-11-23T07:06:09Z

les/utils/limiter.go

+		return process
+	}
+	if value > l.maxValue {
+		l.maxValue = value


It looks a bit hacky to me. It means the base(maxValue) for all other requests might be different. I can understand in order to normalize it we have to adjust the cap.

Why not just passing a "value" in range [0, 1] from the outside? It likes more like a library.

The value can be an arbitrarily high number and it is a lot easier to use if we don't expect the caller to know the highest possible value in advance. Changing maxValue while running is not really a problem because it only screws up probability ratios for a short time, each node will be calculated with the right max value next time. I think this solution is convenient and has practically zero risk.

rjl493456442 · 2020-11-23T07:19:42Z

les/utils/limiter.go

+			l.lock.Unlock()
+			costCh := make(chan float64)
+			request.process <- costCh
+			relCost := <-costCh / float64(l.maxPriorWeight)


It's also hacky. The base for relcost is changing all the time(maxPriorWeight).

Also the <-costCh return is super unclear for the read. What does it mean? What's the relevant value range of the return?

It was the "real cost" of the request (range between 0 and priorWeight) but now it is dropped.

rjl493456442 · 2020-11-23T07:20:47Z

les/utils/limiter.go

+			request.process <- costCh
+			relCost := <-costCh / float64(l.maxPriorWeight)
+			l.lock.Lock()
+			nq.relCost += relCost


I'm not quite sure, maybe we can use assignment? nq.relCost = relCost? It's always reset before the unlock....

rjl493456442 · 2020-11-23T07:36:53Z

les/utils/limiter.go

+		}
+		list = append(list, dropListItem{
+			nq:       nq,
+			priority: w / float64(nq.totalPriorWeight),


It's unnecessary to divide the nq.totalPriorWeight right?

No, it does matter because we iterate on all node queues so it is different each time.

rjl493456442 · 2020-11-23T07:39:48Z

les/utils/limiter.go

+		l.totalPriorWeight -= item.nq.totalPriorWeight
+		// set the last relative cost as if all dropped requests were processed with
+		// the highest possible cost (which equals their prior weight).
+		item.nq.relCost += float64(item.nq.totalPriorWeight)


Why to add the totalPriorWeight directly? In another place https://github.com/ethereum/go-ethereum/pull/21676/files#diff-05f453344a1370c706aca53fb8864362a541daeca84a5c82aecf9344c7af20acR326 it's divided by the base

Right, it was a mistake, now it is simplified and handled correctly.

zsfelfoldi · 2020-11-30T22:47:55Z

The UDP limiter is now in a separate PR, simplified and issues addressed:
#21930
(the cost filter is not needed anymore)

zsfelfoldi requested review from fjl and rjl493456442 as code owners October 8, 2020 02:37

zsfelfoldi added the status:in-progress label Oct 8, 2020

zsfelfoldi force-pushed the les4udp branch 3 times, most recently from 234b976 to 0e9a56e Compare October 12, 2020 18:44

zsfelfoldi force-pushed the les4udp branch 2 times, most recently from 7cf8782 to 33cc672 Compare October 15, 2020 15:03

zsfelfoldi added 6 commits October 26, 2020 20:33

les, p2p: use new discv5

6489666

p2p/discover: add source id and address to talk request handler

fc4f31c

les/utils: implemented CostFilter and Limiter

e134fcb

les/lespay/server: implemented lespay command server

ae1f527

les: implemented capacity query

7d185ce

les: add pre-negotiation query to server pool

33e7a46

zsfelfoldi force-pushed the les4udp branch from 33cc672 to 2cbf48a Compare October 26, 2020 19:41

les: atomic database access for services

b33f95e

zsfelfoldi force-pushed the les4udp branch 2 times, most recently from 14b7791 to 6501fd5 Compare October 31, 2020 14:51

zsfelfoldi added 2 commits November 6, 2020 02:23

les: les/4 protocol extensions

48c7ec9

les: added comments

87bd900

zsfelfoldi force-pushed the les4udp branch from 1c28761 to 87bd900 Compare November 6, 2020 01:24

zsfelfoldi changed the title ~~les: implement lespay udp capacity query (WIP)~~ les: les/4 protocol extensions (WIP) Nov 6, 2020

zsfelfoldi added 5 commits November 9, 2020 14:34

les/lespay: changes CapQuery.AddTokens type to IntOrInf

004a24e

les: handler test for LespayRequestMsg

d150128

les: handler test for CapacityRequest

d09aec0

les/flowcontrol: fixed and simplified capacity update

7c670f1

les: added comments

55aa9cc

zsfelfoldi force-pushed the les4udp branch from 67b9576 to 55aa9cc Compare November 12, 2020 01:47

zsfelfoldi added the pr:review label Nov 12, 2020

zsfelfoldi changed the title ~~les: les/4 protocol extensions (WIP)~~ les: les/4 protocol extensions Nov 12, 2020

rjl493456442 reviewed Nov 23, 2020

View reviewed changes

zsfelfoldi removed the pr:review label Dec 8, 2020

zsfelfoldi closed this Feb 15, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

les: les/4 protocol extensions #21676

les: les/4 protocol extensions #21676

zsfelfoldi commented Oct 8, 2020 •

edited

Loading

fjl commented Oct 14, 2020

rjl493456442 commented Nov 20, 2020

rjl493456442 commented Nov 20, 2020 •

edited

Loading

rjl493456442 commented Nov 20, 2020 •

edited

Loading

rjl493456442 Nov 23, 2020

rjl493456442 Nov 23, 2020

zsfelfoldi Nov 30, 2020

rjl493456442 Nov 23, 2020

zsfelfoldi Nov 30, 2020

rjl493456442 Nov 23, 2020

zsfelfoldi Nov 30, 2020

rjl493456442 Nov 23, 2020

zsfelfoldi Nov 30, 2020

rjl493456442 Nov 23, 2020

zsfelfoldi Nov 30, 2020

rjl493456442 Nov 23, 2020

rjl493456442 Nov 23, 2020

zsfelfoldi Nov 30, 2020

rjl493456442 Nov 23, 2020

rjl493456442 Nov 23, 2020

zsfelfoldi Nov 30, 2020

rjl493456442 Nov 23, 2020

zsfelfoldi Nov 30, 2020

zsfelfoldi commented Nov 30, 2020

les: les/4 protocol extensions #21676

les: les/4 protocol extensions #21676

Conversation

zsfelfoldi commented Oct 8, 2020 • edited Loading

fjl commented Oct 14, 2020

rjl493456442 commented Nov 20, 2020

rjl493456442 commented Nov 20, 2020 • edited Loading

rjl493456442 commented Nov 20, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zsfelfoldi commented Nov 30, 2020

zsfelfoldi commented Oct 8, 2020 •

edited

Loading

rjl493456442 commented Nov 20, 2020 •

edited

Loading

rjl493456442 commented Nov 20, 2020 •

edited

Loading