-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
storage
module: add go docs and minor code quality refactors
#6259
Conversation
@nonsense comments addressed! |
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.
A couple of nits but in general looks good 👍
adaptedAPI = NewSealingAPIAdapter(m.api) | ||
|
||
// Instantiate a precommit policy. | ||
defaultDuration = policy.GetMaxSectorExpirationExtension() - (md.WPoStProvingPeriod * 2) |
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.
Suggest adding a comment to explain this calculation
pcp = sealing.NewBasicPreCommitPolicy(adaptedAPI, defaultDuration, provingBoundary) | ||
|
||
// address selector. | ||
as = func(ctx context.Context, mi miner.MinerInfo, use api.AddrUse, goodFunds, minFunds abi.TokenAmount) (address.Address, abi.TokenAmount, error) { |
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.
Suggest adding a comment explaining what the address selector does
@@ -833,6 +879,8 @@ func (s *WindowPoStScheduler) setSender(ctx context.Context, msg *types.Message, | |||
minGasFeeMsg.GasFeeCap = msg.GasFeeCap | |||
} | |||
|
|||
// goodFunds = funds needed for optimal inclusion probability. |
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.
👍 👍
// This object is the owner of the sealing pipeline. Most of the actual logic | ||
// lives in the storage-sealing module (sealing.Sealing), and the Miner object | ||
// exposes it to the rest of the system by proxying calls. |
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.
This is mostly tech debt, but it's really tricky to untangle storage-sealing from this object, which is why it's here
This absorbs #2340. Refer to the commit log for details.