Renter stoped download early, farmer sending success exchange report

littleskunk commented 6 years ago

Package Versions

Replace the values below using the output from npm list storj. Use npm list -g storj if installed globally.

daemon: 5.1.0, core: 8.1.0, protocol: 1.2.0

Replace the values below using the output from node --version.

v6.11.3

Expected Behavior

Please describe the program's expected behavior. Include an example of your usage code in the back ticks below if applicable.

The renter stoped a 5GB download after 10 seconds.
The farmer should send a download failed exchange report.

Actual Behavior

Please describe the program's actual behavior. Please include any stack traces or log output in the back ticks below.

The farmer is sending and waiting for the renter to close the channel once he received all bytes.
If the renter is closing the channel early the farmer is sending a success report `SHARD_DOWNLOADED`

Steps to Reproduce

Please include the steps the reproduce the issue, numbered below. Include as much detail as possible.

Upload one big shard.
Download that shard but stop the transfer after a few seconds.
Compare farmer and renter exchange report.

braydonf commented 6 years ago

Related code for this is around https://github.com/Storj/core/blob/master/lib/network/shard-server.js#L251

littleskunk commented 6 years ago

Wrong direction. For shard uploads the farmer is checking the shard hash and will send an failed report.

Shard downloads are here: https://github.com/Storj/core/blob/master/lib/network/shard-server.js#L362

braydonf commented 6 years ago

Ah right... there may be a similar issue with uploads?

braydonf commented 6 years ago

So to send an end event is happening on the read stream in that case?

littleskunk commented 6 years ago

Shard uploads are checked in all directions. The farmer will not store more bytes than written in the contract. The farmer will not store any wrong shard hashes. That includes too small shards.

littleskunk commented 6 years ago

You are correct. For shard download we should already send the correct report here: https://github.com/Storj/core/blob/master/lib/network/shard-server.js#L429

I send the renter a message. Lets see what his plan was.

braydonf commented 6 years ago

It doesn't look like an exchange report is sent with early termination https://github.com/Storj/core/blob/master/lib/network/shard-server.js#L425

braydonf commented 6 years ago

_handleTransferFinish in routeRetrieval may need to track success status?

littleskunk commented 6 years ago

Oh nice one. That is a memory leak btw.

I found one on my logfile

{"level":"warn","message":"channel terminated early (possibly by client)","timestamp":"2017-10-02T22:44:03.555Z"}

In this case the exchange report is missing and I am not sure the transfered bytes will be deleted. That is a different issue for another day.

Now we are close to 100% sure that the problem is on the renter side. I will wait for the renter response but most likely close this issue.

littleskunk commented 6 years ago

I am fine with additional checks. As farmer I would prefer to avoid any false positives. It would be nice if the download would be counted as failed even if the renter behavior was strange. Long term this is better for a robust reputation system. What can we do if renter and farmer are sending different exchange reports? Better avoid that situation early.

braydonf commented 6 years ago

The client may terminate the transfer if it's too slow, by the LOW_SPEED_LIMIT setting. So that could be a source of early termination, other than other network issues, or if the transfer is canceled.

littleskunk commented 6 years ago

I have only 7 downloads last month. 4 canceld by the renter at the 10 seconds mark. The other 3 successfully in less than 10 seconds. My first bet was the LOW_SPEED_LIMIT. However that would be a different issue. Lets focus on the this one.

There is no way I can deliver 5GB in 10 seconds. 3 times in a row. -> The renter closed the channel early. My farmer was sending a SHARD_DOWNLOADED for all 3.

littleskunk commented 6 years ago

I have an easy fix for this. The same way the shard upload is counting the transfered bytes I added a counter for the downloaded bytes. On closing the channel I compare the transfered bytes with the contract size.

{"level":"info","message":"Shard download started hash 588a3bd69f48401e66c5f6c4a1c2c9b7650e4e82 size 25000001","timestamp":"2018-04-02T02:02:34.195Z"}
{"level":"info","message":"Shard download failed hash 588a3bd69f48401e66c5f6c4a1c2c9b7650e4e82 size 14155776","timestamp":"2018-04-02T02:02:44.122Z"}

Should I open a pull request or is the farmer outdated anyway and we will not publish any improvements on the old network?

RichardLitt commented 5 years ago

👋 Hey! Thanks for this contribution. Apologies for the delay in responding!

We've decided to rearchitect Storj, so that we can scale better. You can read more about this decision here. This means that we are entirely focused on v3 at the moment, in the storj/storj repository. Our white paper for v3 is coming very, very soon - follow along on the blog and in our Rocketchat.

As this repository is part of the v2 network, we're no longer maintaining this repository. I am going to close this for now. If you have any questions, I encourage you to jump on Rocketchat and ask them there. Thanks!

storj-archived / core