Compute new collateral for renewals in worker instead of the autopilot #811
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ChrisSchinnerl
force-pushed
the
chris/max-collateral-renew
branch
from
b56614d to
4ad17ae
Compare
peterjan
reviewed
Dec 12, 2023
autopilot/contractor.go
Outdated
| @@ -1387,8 +1394,8 @@ func (c *contractor) renewContract(ctx context.Context, w Worker, ci contractInf | |||
| *budget = budget.Sub(renterFunds) | |||
|
|
|||
| // persist the contract | |||
| contractPrice := newRevision.Revision.MissedHostPayout().Sub(newCollateral) | |||
| renewedContract, err := c.ap.bus.AddRenewedContract(ctx, newRevision, contractPrice, renterFunds, cs.BlockHeight, fcid, api.ContractStatePending) | |||
| newCollateral := resp.Contract.Revision.MissedHostPayout().Sub(resp.ContractPrice) | |||
There was a problem hiding this comment.
nit maybe move this down so it's clear this is used for logging purposes only
peterjan
reviewed
Dec 12, 2023
| } | ||
|
|
||
| // RHPRenewResponse is the response type for the /rhp/renew endpoint. | ||
| RHPRenewResponse struct { | ||
| Error string `json:"error"` | ||
| ContractID types.FileContractID `json:"contractID"` | ||
| Contract rhpv2.ContractRevision `json:"contract"` | ||
| ContractPrice types.Currency `json:"contractPrice"` |
There was a problem hiding this comment.
Does it make sense to drop ContractID from RHPRenewResponse ? It's duplicated info.
Does it make sense to return the renewCollateral directly instead of the ContractPrice?
There was a problem hiding this comment.
The ContractPrice is the thing we actually need for the next API call. So I'm returning it instead of the thing we only need for logging. At the end of the day it doesn't really matter cause the endpoint will be removed with rhpv4 anyway once we remove anything related to contracts to the bus.
peterjan
approved these changes
Dec 12, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
That way we can do so after locking the contract to avoid exceeding the max collateral when uploading at the same time.
As a side - effect, since we only pass a pricetable to
RPCRenew, we also use rhpv3 estimates now.