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

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Nuhvi
Copy link

@Nuhvi Nuhvi commented Nov 28, 2022

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
Copy link
Contributor

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
Copy link
Author

Nuhvi commented Nov 28, 2022

@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
Copy link
Contributor

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
Copy link
Author

Nuhvi commented Nov 28, 2022

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
Copy link
Contributor

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

index.js Outdated Show resolved Hide resolved
@Nuhvi
Copy link
Author

Nuhvi commented Nov 28, 2022

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!

@Nuhvi Nuhvi force-pushed the commit-local-mutable-record branch from 31b7d8f to e50e423 Compare November 28, 2022 09:58
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

Successfully merging this pull request may close these issues.

2 participants