Skip to content
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

[wip] feat: sturdypost: WindowPoSt Submit #11380

Merged
merged 12 commits into from
Nov 8, 2023

Conversation

magik6k
Copy link
Contributor

@magik6k magik6k commented Nov 3, 2023

Related Issues

Closes #11379

Proposed Changes

  • Add a util for sending messages through HarmonyDB
    • To be used by all message-sending tasks, abstracts away some chain interaction specifics
  • Implement a Submit task
    • WIP

Additional Info

Checklist

Before you mark the PR ready for review, please make sure that:

  • Commits have a clear commit message.
  • PR title is in the form of of <PR type>: <area>: <change being made>
    • example: fix: mempool: Introduce a cache for valid signatures
    • PR type: fix, feat, build, chore, ci, docs, perf, refactor, revert, style, test
    • area, e.g. api, chain, state, market, mempool, multisig, networking, paych, proving, sealing, wallet, deps
  • If the PR affects users (e.g., new feature, bug fix, system requirements change), update the CHANGELOG.md and add details to the UNRELEASED section.
  • New features have usage guidelines and / or documentation updates in
  • Tests exist for new functionality or change in behavior
  • CI is green

@@ -24,5 +24,11 @@ create table wdpost_proofs
partition bigint not null,
submit_at_epoch bigint not null,
submit_by_epoch bigint not null,
proof_message bytea
proof_message bytea,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should rename this to proof_params before merging

Copy link
Collaborator

@snadrus snadrus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

provider/lpwindow/submit_task.go Outdated Show resolved Hide resolved
@magik6k magik6k marked this pull request as ready for review November 7, 2023 14:34
@magik6k magik6k requested a review from a team as a code owner November 7, 2023 14:34
@magik6k magik6k requested a review from snadrus November 7, 2023 14:34
@@ -64,10 +65,13 @@ func Register(db *harmonydb.DB, hostnameAndPort string) (*Reg, error) {
SELECT id FROM inserted;
`, hostnameAndPort, reg.Cpu, reg.Ram, reg.Gpu).Scan(&ownerID)
if err != nil {
return nil, err
return nil, xerrors.Errorf("inserting machine entry: %w", err)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use fmt.Errorf which has %w now?

as: as,
}

if err := pcs.AddHandler(res.processHeadChange); err != nil {
Copy link
Collaborator

@snadrus snadrus Nov 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens for these case when the db entry is added after its submit epoch has started (potentially common for WinningPoSt)
-- a "check_now()" type of function called from the proof creator could help here.

GasEstimateGasPremium(_ context.Context, nblocksincl uint64, sender address.Address, gaslimit int64, tsk types.TipSetKey) (types.BigInt, error)
}

type WdPostSubmitTask struct {
Copy link
Collaborator

@snadrus snadrus Nov 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How generic could this be made WRT having it support WindowPoSt and/or sealing?

I ask because the database tables will be written after this goes out and any attempt to make it generic would involve data migration or an interface to swap out submit types that use different tables.

@snadrus snadrus merged commit 8ecee2d into feat/sturdypost Nov 8, 2023
@snadrus snadrus deleted the feat/lp-wdpost-submit branch November 8, 2023 16:57
@magik6k magik6k mentioned this pull request Nov 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants