holepunchto / hyperdht

The DHT powering Hyperswarm
https://docs.holepunch.to
MIT License
323 stars 46 forks source link

feat(mutableGet): commit local mutable record after it is dropped (unlike refresh) #110

Open Nuhvi opened 1 year ago

Nuhvi commented 1 year ago

This is helpful for applications that have different caching tolerance than the 20 min before mutable records are dropped.

For example, if an application is ok with a 60min TTL, then it doesn't make sense to keep refreshing values 3 times as often.

This PR will allow such applications to re-commit locally cached records at their leisure.

mafintosh commented 1 year ago

Not sure I follow the logic can you expand a bit? Only the 20 closest nodes of the entire DHT can (effectively) pick the caching time, unless you get lucky and hit a random node on the way (lower the chance the bigger the dht)

Nuhvi commented 1 year ago

@mafintosh Didn't mean changing the caching time, I meant that applications might want to refresh a value every 60 minutes instead of every 20 minutes, but by then, the value would be dropped, and the current mutableGet implementation won't allow committing the record that the application already knows about and saved either in memory or long term storage.

The alternative is to change mutablePut to allow api like dht.mutablePut(publicKey, value, { signature}, if mutableGet returned null, but that is a waste because mutableGet already got reply.token, so this PR tries to use that to commit records that were already dropped, or never committed but somehow received from the author off-band, which is useful for example for committing records without exposing the keypair to an internet-enabled device.

mafintosh commented 1 year ago

Sorry I don't follow. Can you explain in super simple terms what this does? As I read the code it adding an option to get?

Nuhvi commented 1 year ago

Yes, it is adding value and signature options to mutableGet, so in case the nearest 20 nodes have no record, mutableGet can still create a signed record and commit it to these nodes.

mafintosh commented 1 year ago

Ie “refresh with this value if no other value exist” ?

Nuhvi commented 1 year ago

Ie “refresh with this value if no other value exist” ?

Yes, and set this new value if no newer value exists (but I don't have the keypair).

Will submit a better unit test in a minute to highlight that use case.

Edit: Updated unit test, hopefully the purpose of this PR is a bit more clear!