ethereum / go-ethereum

Go implementation of the Ethereum protocol
https://geth.ethereum.org
GNU Lesser General Public License v3.0
47.14k stars 19.95k forks source link

p2p/discv5 uses monotonic clock reading in Expiration field #17468

Closed gsalgado closed 5 years ago

gsalgado commented 6 years ago

The ticketToPong() function uses the ticket's issueTime, which is a monotonic clock reading, as pong.Expiration, but AIUI that is a meaningless value outside of the geth process itself, and is definitely not the seconds since epoch that I'd expect to find in pong.Expiration

fjl commented 6 years ago

Yes, that's a bug. I will send a PR to fix this.

zsfelfoldi commented 5 years ago

This is not a bug. It is true that this is a local monotonic clock which is meaningless for another client but this value is only interpreted by the issuer itself when the other party returns with the ticket after the wait period. The issuer could also remember it but the idea was that the issuer should not need to store anything about tickets in order to avoid spam attacks by requesting tickets. So the local issue time is stored in the ticket and signed, then checked when receiving the ticket back. The other party only uses the wait period which it counts relative to its own local time when receiving the ticket.

zsfelfoldi commented 5 years ago

It is misleading though that the field is called Expiration which it is definitely not. We should change it to IssueTime and document it properly that it is not intended for use by anybody other than the issuer. We are heavily reworking discv5 very soon anyway, I'll properly document the whole protocol after that. I did not document the current version because it was intended to be a hacked-together experiment and it is unfortunate that LES still relies on this version.