test: add failing put/get test with salted content

[robertkowalski]

when we store data with a salt, and get back a hash to retrieve
the data later, the data is not found via the hash any more in
8.4.

[robertkowalski]

update: this also fails with test vector 2 from http://bittorrent.org/beps/bep_0044.html, so it seems like a bug

[chr15m]

@robertkowalski I believe these tests will start working again if you include the salt parameter in the get options. The Bittorrent specification expects that clients will know what salt they are looking for and so we should expect this from WebTorrent clients too and the tests should reflect this.

Specifically on these two lines:

7252ff2#diff-3309f8e8309d79dd0ea3ba112b4c18e0R110

8ea849c#diff-c48b89eba3c2629431adc02e9d8a71e1R45

You should be passing opts which looks like {"salt": Buffer.from('foobar')} and {"salt": toEncode.salt} respectively.

I'll try to make this change and test later today.

[chr15m]

Ok, I think I have figured out what is going on.

The tests from @robertkowalski are failing not because of the salt change but because of ac3f8cb. What that commit does is change the default behaviour to not cache DHT mutable get/put values. I think this breaks the DHT tests because they assume the locally cached value. @feross there is actually no point in merging ac3f8cb since users can specify whether they want to cache or not cache using opts.cache as per #195 which you have also merged.

The tests themselves however, I think only work because the locally cached value is cached with the salt parameter. This is not true of real-world Bittorrent nodes which will not usually send the salt parameter back. So I would still upgrade the tests to be spec compliant and specify salt during get.

Recommendation:

• Back out ac3f8cb since users can choose whether or not to cache with opts.cache.
• Upgrade the tests from @robertkowalski to add salt during get.

Update: I was wrong about this. The tests themselves are buggy. See #200 for fixed versions of @robertkowalski's tests.

[robertkowalski]

@chr15m thanks for your work.

you fixed the test by passing the salt to the get function. i don't see anything in the spec that passing the salt on a get is required. can you point me to the sentences / sections?

[chr15m]

@robertkowalski No I didn't fix the test by passing the salt to the get function. I removed that here because it is superfluous to the fix:

cbf06d4#diff-3309f8e8309d79dd0ea3ba112b4c18e0L122

However the only reason this works is because of the way the test system works. In the real world you will need to pass salt to get.

The reason the spec does not say anything about passing salt to get is because the spec is concerned with the network protocol - what is sent over the wire. You do not need to send salt over the wire during get and nodes do not do this.

However you must be able to validate the responses from other DHT nodes during the get process. The only way to verify the signature is if you know the salt that went into it. The spec does not say to send salt with get responses and real-world Bittorrent clients do not send the salt. So the receiving client making the get request must have some way of knowing the salt.

So the reason we must pass salt to get is so that when we receive those responses from remote nodes (which do not send back salt) and we need to validate the signature, we need to know the salt.

Sorry if this is not very clear. Tomorrow morning I will try to draw a diagram.

[robertkowalski]

@chr15m thanks, makes sense now. superceded by #200