-
Notifications
You must be signed in to change notification settings - Fork 9
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
RPCCost (RHPv2) #123
RPCCost (RHPv2) #123
Conversation
I'm not entirely sure why the RHP stuff in I think my preference is:
|
58fffd9
to
5cad92d
Compare
Yeah I saw you removed However, since you mention we ideally go for exact cost calculations. Would you prefer changing some of those functions entirely to accept |
b7b4c57
to
21b5bcd
Compare
For rhpv3 we could/should update the cost methods on the |
21b5bcd
to
205a0ff
Compare
205a0ff
to
66c5800
Compare
Add(settings.UploadBandwidthPrice.Mul64(SectorSize)). | ||
Add(settings.DownloadBandwidthPrice.Mul64(128 * 32)) // proof | ||
collateral = settings.Collateral.Mul64(SectorSize).Mul64(storageDuration) | ||
// add some leeway to reduce chance of host rejecting |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@n8maninger hostd
stopped doing it so I copied that behaviour, just wanted to draw attention to it to ensure it's safe to do so
return settings.BaseRPCPrice. | ||
Add(settings.DownloadBandwidthPrice.Mul64(n * 32)). // roots | ||
Add(settings.DownloadBandwidthPrice.Mul64(128 * 32)) // proof | ||
type RPCCost struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Functionally this type matches the ResourceCost
in RHPv3, we could move this into the types
package maybe and use the one type. I didn't want to do that yet mostly because of the documentation in ResourceCost
which is tailored to the MDM... Maybe it's fine to just have both, for the same reason hostd
had its own internal rpcCost
type.
rhp/v2/rhp.go
Outdated
case sec.Length == 0: | ||
return RPCCost{}, errors.New("length cannot be zero") | ||
case proof && (sec.Offset%LeafSize != 0 || sec.Length%LeafSize != 0): | ||
return RPCCost{}, errors.New("offset and length must be multiples of SegmentSize when requesting a Merkle proof") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
*LeafSize
This PR adds the
rpcCost
type fromhostd
. We can move it over tocore
and get rid of some of the very rough estimates for proof sizes and replace them by exact values we get fromDiffProofSize
.