NebulousLabs / Sia

Blockchain-based marketplace for file storage. Project has moved to GitLab: https://gitlab.com/NebulousLabs/Sia
https://sia.tech
MIT License
2.71k stars 442 forks source link

Updates on Spend Reporting PR 3087 #3131

Closed MSevey closed 5 years ago

MSevey commented 6 years ago

Removed old contracts from Export to avoid breaking backwards compatibility.

Updated releaseblock calculation.

@DavidVorick if there are other edits you find during your review please add TODO's to this PR so we can merge them in.

DavidVorick commented 6 years ago

And you need to update the api documentation for the /renter/contracts endpoint to reflect that old contracts are now returned as well.

And... we should consider having the old contracts get returned as a flag instead of always getting returned, because over time that will build up to be a bunch of data.

MSevey commented 6 years ago

And... we should consider having the old contracts get returned as a flag instead of always getting returned, because over time that will build up to be a bunch of data.

Are you saying to look for the flag in renterContractsHandler so that only the active contracts are returned unless the flag is provided? So adding like an oldContracts bool or something?

MSevey commented 6 years ago

I also went ahead and renamed OldContracts to ExpiredContracts since active and expired make more sense than active and old.

DavidVorick commented 6 years ago

Yeah, add a bool so that it only returns the expired contract if a flag is passed in, that way long running renters don't have to worry about getting 50,000 returned objects when they make a query.

MSevey commented 6 years ago

Now the thought is to leave just the one field Contracts and add flags to return the type of contracts requested. Here is a proposal of the flags:

active - contracts that are in the current period and are goodForRenew and goodForUpload (SH >= CP)

inactive - contracts in the current period that are !goodForRenew or !goodForUpload (SH >= CP)

expired - contracts not in the current period (ie EH <= CP)

all - all contracts

The functionality could be that there is no default contracts returned, so if no flag is provided then no contracts are return. This would mean that you would only get the contracts for the flag you submitted and if you want for example both active and inactive contracts then you would need to submit both flags.

DavidVorick commented 6 years ago

That distinction looks good to me. inactive would include contracts that have been renewed due to depletion? (I think it would, as they would be !goodForRewew)

MSevey commented 6 years ago

@DavidVorick currently I'm running into the following edge cases that I need help deciding how to handle.

For reference, here is how I am currently defining the contracts. From the renter's Contracts:

In both cases c.EndHeight > currentPeriod is probably redundant since they shouldn't be in Contracts if that were false but I'm leaving it in there for now.

From the renter's OldContacts:

As you might guess from looking at the expired vs inactive for the OldContracts when the allowance is cancelled the currentPeriod is equal to zero and both of those evaluate to true. I can't think of a good way to distinguish between the two if the allowance is cancelled and the currentPeriod is 0. Should we just make the definition that if there is no allowance all the OldContracts are seen as expired?

MSevey commented 5 years ago

test-long is passing locally

MSevey commented 5 years ago

siac renter contracts

image

MSevey commented 5 years ago

siac renter contracts -A image

MSevey commented 5 years ago

siac if no contracts have been formed image

MSevey commented 5 years ago

fixed column headers image