From 20bf029c34fccc622ddb47da26acee1a777669a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Magiera?= Date: Fri, 6 Sep 2024 16:54:05 +0200 Subject: [PATCH] address review --- cmd/curio/unseal.go | 25 ++++++++++++----- harmony/harmonytask/harmonytask.go | 10 +++---- lib/ffi/cunative/decode_snap.go | 45 +++++++++++++++--------------- lib/ffi/sdr_funcs.go | 2 +- lib/proof/datacid.go | 2 ++ lib/storiface/filetype.go | 2 +- tasks/scrub/task_scrub_commd.go | 2 +- tasks/snap/task_encode.go | 2 +- tasks/snap/task_movestorage.go | 2 +- tasks/snap/task_prove.go | 2 +- tasks/snap/task_submit.go | 2 +- tasks/unseal/task_unseal_decode.go | 2 ++ 12 files changed, 56 insertions(+), 42 deletions(-) diff --git a/cmd/curio/unseal.go b/cmd/curio/unseal.go index 92e86cd1b..7c962b183 100644 --- a/cmd/curio/unseal.go +++ b/cmd/curio/unseal.go @@ -364,7 +364,7 @@ var listUnsealPipelineCmd = &cli.Command{ } row := []string{ - "f0" + strconv.FormatInt(spID, 10), + must.One(address.NewIDAddress(uint64(spID))).String(), strconv.FormatInt(sectorNumber, 10), strconv.FormatInt(regSealProof, 10), formatNullableBool(targetUnsealState), @@ -393,11 +393,22 @@ var listUnsealPipelineCmd = &cli.Command{ var setTargetUnsealStateCmd = &cli.Command{ Name: "set-target-state", Usage: "Set the target unseal state for a sector", - ArgsUsage: " ", + ArgsUsage: " ", Description: `Set the target unseal state for a specific sector. - : The storage provider ID + : The storage provider ID : The sector number - : The target state (true, false, or none)`, + : The target state (true, false, or none) + + The unseal target state indicates to curio how an unsealed copy of the sector should be maintained. + If the target state is true, curio will ensure that the sector is unsealed. + If the target state is false, curio will ensure that there is no unsealed copy of the sector. + If the target state is none, curio will not change the current state of the sector. + + Currently when the curio will only start new unseal processes when the target state changes from another state to true. + + When the target state is false, and an unsealed sector file exists, the GC mark step will create a removal mark + for the unsealed sector file. The file will only be removed after the removal mark is accepted. +`, Action: func(cctx *cli.Context) error { if cctx.Args().Len() != 3 { return cli.ShowSubcommandHelp(cctx) @@ -469,10 +480,10 @@ func formatNullableBool(v *bool) string { var unsealCheckCmd = &cli.Command{ Name: "check", - Usage: "Check data in unsealed sector files", - ArgsUsage: " ", + Usage: "Check data integrity in unsealed sector files", + ArgsUsage: " ", Description: `Create a check task for a specific sector, wait for its completion, and output the result. - : The storage provider ID + : The storage provider ID : The sector number`, Action: func(cctx *cli.Context) error { if cctx.Args().Len() != 2 { diff --git a/harmony/harmonytask/harmonytask.go b/harmony/harmonytask/harmonytask.go index d1a1b4e82..05cf75a67 100644 --- a/harmony/harmonytask/harmonytask.go +++ b/harmony/harmonytask/harmonytask.go @@ -110,10 +110,6 @@ type MaxCounter struct { } func (m *MaxCounter) max() int { - if m == nil { - return 0 - } - return m.N } @@ -132,7 +128,7 @@ func (m *MaxCounter) add(n int) { } func Max(n int) *MaxCounter { - return &MaxCounter{N: n, current: new(atomic.Int32)} + return &MaxCounter{N: n, current: new(atomic.Int32), currentThis: new(atomic.Int32)} } // AddTaskFunc is responsible for adding a task's details "extra info" to the DB. @@ -198,6 +194,10 @@ func New( TaskTypeDetails: c.TypeDetails(), TaskEngine: e, } + if h.Max == nil { + h.Max = Max(0) + } + if Registry[h.TaskTypeDetails.Name] == nil { return nil, fmt.Errorf("task %s not registered: var _ = harmonytask.Reg(t TaskInterface)", h.TaskTypeDetails.Name) } diff --git a/lib/ffi/cunative/decode_snap.go b/lib/ffi/cunative/decode_snap.go index c6a1f1c3b..25b346c3d 100644 --- a/lib/ffi/cunative/decode_snap.go +++ b/lib/ffi/cunative/decode_snap.go @@ -252,7 +252,7 @@ func Phi(commDNew, commROld BytesLE) (B32le, error) { inputB := bigIntLE(commROld) input := []*big.Int{inputA, inputB} - cons, err := poseidon.GenPoseidonConstants[*CursedPoseidonGenRandomnessElement](3) + cons, err := poseidon.GenPoseidonConstants[*CustomDomainSepTagElement](3) if err != nil { return [32]byte{}, err } @@ -273,7 +273,7 @@ func rho(phi B32le, high uint32) (*fr.Element, error) { inputB := new(big.Int).SetUint64(uint64(high)) input := []*big.Int{inputA, inputB} - cons, err := poseidon.GenPoseidonConstants[*CursedPoseidonGenRandomnessElement](3) + cons, err := poseidon.GenPoseidonConstants[*CustomDomainSepTagElement](3) if err != nil { return nil, err } @@ -378,14 +378,13 @@ func bigIntLE(in BytesLE) *big.Int { return new(big.Int).SetBytes(b) } -///// -// Sanity lost beyond this point - -type CursedPoseidonGenRandomnessElement struct { +// CustomDomainSepTagElement is a custom element which overrides SetString used by the poseidon hash function to set +// the default hardcoded DST. We hijack the SetString function to set the DST to the hardcoded value needed for Snap. +type CustomDomainSepTagElement struct { *fr.Element } -func (c *CursedPoseidonGenRandomnessElement) SetUint64(u uint64) *CursedPoseidonGenRandomnessElement { +func (c *CustomDomainSepTagElement) SetUint64(u uint64) *CustomDomainSepTagElement { if c.Element == nil { c.Element = new(fr.Element) } @@ -394,7 +393,7 @@ func (c *CursedPoseidonGenRandomnessElement) SetUint64(u uint64) *CursedPoseidon return c } -func (c *CursedPoseidonGenRandomnessElement) SetBigInt(b *big.Int) *CursedPoseidonGenRandomnessElement { +func (c *CustomDomainSepTagElement) SetBigInt(b *big.Int) *CustomDomainSepTagElement { if c.Element == nil { c.Element = new(fr.Element) } @@ -403,7 +402,7 @@ func (c *CursedPoseidonGenRandomnessElement) SetBigInt(b *big.Int) *CursedPoseid return c } -func (c *CursedPoseidonGenRandomnessElement) SetBytes(bytes []byte) *CursedPoseidonGenRandomnessElement { +func (c *CustomDomainSepTagElement) SetBytes(bytes []byte) *CustomDomainSepTagElement { if c.Element == nil { c.Element = new(fr.Element) } @@ -412,7 +411,7 @@ func (c *CursedPoseidonGenRandomnessElement) SetBytes(bytes []byte) *CursedPosei return c } -func (c *CursedPoseidonGenRandomnessElement) BigInt(b *big.Int) *big.Int { +func (c *CustomDomainSepTagElement) BigInt(b *big.Int) *big.Int { if c.Element == nil { c.Element = new(fr.Element) } @@ -420,7 +419,7 @@ func (c *CursedPoseidonGenRandomnessElement) BigInt(b *big.Int) *big.Int { return c.Element.BigInt(b) } -func (c *CursedPoseidonGenRandomnessElement) SetOne() *CursedPoseidonGenRandomnessElement { +func (c *CustomDomainSepTagElement) SetOne() *CustomDomainSepTagElement { if c.Element == nil { c.Element = new(fr.Element) } @@ -429,7 +428,7 @@ func (c *CursedPoseidonGenRandomnessElement) SetOne() *CursedPoseidonGenRandomne return c } -func (c *CursedPoseidonGenRandomnessElement) SetZero() *CursedPoseidonGenRandomnessElement { +func (c *CustomDomainSepTagElement) SetZero() *CustomDomainSepTagElement { if c.Element == nil { c.Element = new(fr.Element) } @@ -438,7 +437,7 @@ func (c *CursedPoseidonGenRandomnessElement) SetZero() *CursedPoseidonGenRandomn return c } -func (c *CursedPoseidonGenRandomnessElement) Inverse(e *CursedPoseidonGenRandomnessElement) *CursedPoseidonGenRandomnessElement { +func (c *CustomDomainSepTagElement) Inverse(e *CustomDomainSepTagElement) *CustomDomainSepTagElement { if c.Element == nil { c.Element = new(fr.Element) } @@ -447,7 +446,7 @@ func (c *CursedPoseidonGenRandomnessElement) Inverse(e *CursedPoseidonGenRandomn return c } -func (c *CursedPoseidonGenRandomnessElement) Set(e *CursedPoseidonGenRandomnessElement) *CursedPoseidonGenRandomnessElement { +func (c *CustomDomainSepTagElement) Set(e *CustomDomainSepTagElement) *CustomDomainSepTagElement { if c.Element == nil { c.Element = new(fr.Element) } @@ -456,7 +455,7 @@ func (c *CursedPoseidonGenRandomnessElement) Set(e *CursedPoseidonGenRandomnessE return c } -func (c *CursedPoseidonGenRandomnessElement) Square(e *CursedPoseidonGenRandomnessElement) *CursedPoseidonGenRandomnessElement { +func (c *CustomDomainSepTagElement) Square(e *CustomDomainSepTagElement) *CustomDomainSepTagElement { if c.Element == nil { c.Element = new(fr.Element) } @@ -465,7 +464,7 @@ func (c *CursedPoseidonGenRandomnessElement) Square(e *CursedPoseidonGenRandomne return c } -func (c *CursedPoseidonGenRandomnessElement) Mul(e2 *CursedPoseidonGenRandomnessElement, e *CursedPoseidonGenRandomnessElement) *CursedPoseidonGenRandomnessElement { +func (c *CustomDomainSepTagElement) Mul(e2 *CustomDomainSepTagElement, e *CustomDomainSepTagElement) *CustomDomainSepTagElement { if c.Element == nil { c.Element = new(fr.Element) } @@ -474,7 +473,7 @@ func (c *CursedPoseidonGenRandomnessElement) Mul(e2 *CursedPoseidonGenRandomness return c } -func (c *CursedPoseidonGenRandomnessElement) Add(e2 *CursedPoseidonGenRandomnessElement, e *CursedPoseidonGenRandomnessElement) *CursedPoseidonGenRandomnessElement { +func (c *CustomDomainSepTagElement) Add(e2 *CustomDomainSepTagElement, e *CustomDomainSepTagElement) *CustomDomainSepTagElement { if c.Element == nil { c.Element = new(fr.Element) } @@ -483,7 +482,7 @@ func (c *CursedPoseidonGenRandomnessElement) Add(e2 *CursedPoseidonGenRandomness return c } -func (c *CursedPoseidonGenRandomnessElement) Sub(e2 *CursedPoseidonGenRandomnessElement, e *CursedPoseidonGenRandomnessElement) *CursedPoseidonGenRandomnessElement { +func (c *CustomDomainSepTagElement) Sub(e2 *CustomDomainSepTagElement, e *CustomDomainSepTagElement) *CustomDomainSepTagElement { if c.Element == nil { c.Element = new(fr.Element) } @@ -492,7 +491,7 @@ func (c *CursedPoseidonGenRandomnessElement) Sub(e2 *CursedPoseidonGenRandomness return c } -func (c *CursedPoseidonGenRandomnessElement) Cmp(x *CursedPoseidonGenRandomnessElement) int { +func (c *CustomDomainSepTagElement) Cmp(x *CustomDomainSepTagElement) int { if c.Element == nil { c.Element = new(fr.Element) } @@ -500,10 +499,10 @@ func (c *CursedPoseidonGenRandomnessElement) Cmp(x *CursedPoseidonGenRandomnessE return c.Element.Cmp(x.Element) } -func (c *CursedPoseidonGenRandomnessElement) SetString(s string) (*CursedPoseidonGenRandomnessElement, error) { +func (c *CustomDomainSepTagElement) SetString(s string) (*CustomDomainSepTagElement, error) { if s == "3" { - whatTheFuck := "0000000000010000000000000000000000000000000000000000000000000000" - dstLE := must.One(hex.DecodeString(whatTheFuck)) + genRandomnessDST := "0000000000010000000000000000000000000000000000000000000000000000" + dstLE := must.One(hex.DecodeString(genRandomnessDST)) inverted := make([]byte, len(dstLE)) for i := 0; i < len(dstLE); i++ { inverted[i] = dstLE[len(dstLE)-1-i] @@ -522,4 +521,4 @@ func (c *CursedPoseidonGenRandomnessElement) SetString(s string) (*CursedPoseido return c, nil } -var _ poseidon.Element[*CursedPoseidonGenRandomnessElement] = &CursedPoseidonGenRandomnessElement{} +var _ poseidon.Element[*CustomDomainSepTagElement] = &CustomDomainSepTagElement{} diff --git a/lib/ffi/sdr_funcs.go b/lib/ffi/sdr_funcs.go index 6a85c6560..3a8330d13 100644 --- a/lib/ffi/sdr_funcs.go +++ b/lib/ffi/sdr_funcs.go @@ -162,7 +162,7 @@ func (sb *SealCalls) GenerateSDR(ctx context.Context, taskID harmonytask.TaskID, } intoPath := storiface.PathByType(paths, into) - intoTemp := intoPath + ".tmp" + intoTemp := intoPath + storiface.TempSuffix // make sure the cache dir is empty if err := os.RemoveAll(intoPath); err != nil { diff --git a/lib/proof/datacid.go b/lib/proof/datacid.go index 2afca2743..299ca90b3 100644 --- a/lib/proof/datacid.go +++ b/lib/proof/datacid.go @@ -34,6 +34,8 @@ DataCidWriter is used as follows: cc := new(DataCidWriter) _, err = io.Copy(cc, f) dc, err := cc.Sum() + +This computes CommP / PieceCID from a stream, also returns piece and payload sizes. */ type DataCidWriter struct { len int64 diff --git a/lib/storiface/filetype.go b/lib/storiface/filetype.go index 0d5ccd364..07fab509d 100644 --- a/lib/storiface/filetype.go +++ b/lib/storiface/filetype.go @@ -114,7 +114,7 @@ var FsOverheadFinalized = map[SectorFileType]int{ // TypeFromString converts a string to a SectorFileType type SectorFileType int -// TempSuffix is appended to file names when thef are worked on before being atomically moved to their final location. +// TempSuffix is appended to file names when they are worked on before being atomically moved to their final location. // Local Path GC should be aware of this suffix and have adequate cleanup logic. const TempSuffix = ".tmp" diff --git a/tasks/scrub/task_scrub_commd.go b/tasks/scrub/task_scrub_commd.go index 2e2dc72cf..04fc21391 100644 --- a/tasks/scrub/task_scrub_commd.go +++ b/tasks/scrub/task_scrub_commd.go @@ -130,7 +130,7 @@ func (c *ScrubCommDTask) schedule(ctx context.Context, taskFunc harmonytask.AddT SectorNumber int64 `db:"sector_number"` } - err := c.db.Select(ctx, &checks, ` + err := tx.Select(&checks, ` SELECT check_id, sp_id, sector_number FROM scrub_unseal_commd_check WHERE task_id IS NULL LIMIT 20 diff --git a/tasks/snap/task_encode.go b/tasks/snap/task_encode.go index c66c48f31..30a7ec32f 100644 --- a/tasks/snap/task_encode.go +++ b/tasks/snap/task_encode.go @@ -157,7 +157,7 @@ func (e *EncodeTask) schedule(ctx context.Context, taskFunc harmonytask.AddTaskF SectorNumber int64 `db:"sector_number"` } - err := e.db.Select(ctx, &tasks, `SELECT sp_id, sector_number FROM sectors_snap_pipeline WHERE data_assigned = true AND after_encode = FALSE AND task_id_encode IS NULL`) + err := tx.Select(&tasks, `SELECT sp_id, sector_number FROM sectors_snap_pipeline WHERE data_assigned = true AND after_encode = FALSE AND task_id_encode IS NULL`) if err != nil { return false, xerrors.Errorf("getting tasks: %w", err) } diff --git a/tasks/snap/task_movestorage.go b/tasks/snap/task_movestorage.go index 4970f336d..c45387bd7 100644 --- a/tasks/snap/task_movestorage.go +++ b/tasks/snap/task_movestorage.go @@ -112,7 +112,7 @@ func (m *MoveStorageTask) schedule(ctx context.Context, taskFunc harmonytask.Add SectorNumber int64 `db:"sector_number"` } - err := m.db.Select(ctx, &tasks, `SELECT sp_id, sector_number FROM sectors_snap_pipeline WHERE after_encode = TRUE AND after_prove = TRUE AND after_move_storage = FALSE AND task_id_move_storage IS NULL`) + err := tx.Select(&tasks, `SELECT sp_id, sector_number FROM sectors_snap_pipeline WHERE after_encode = TRUE AND after_prove = TRUE AND after_move_storage = FALSE AND task_id_move_storage IS NULL`) if err != nil { return false, xerrors.Errorf("getting tasks: %w", err) } diff --git a/tasks/snap/task_prove.go b/tasks/snap/task_prove.go index 834014d95..e17abe591 100644 --- a/tasks/snap/task_prove.go +++ b/tasks/snap/task_prove.go @@ -147,7 +147,7 @@ func (p *ProveTask) schedule(ctx context.Context, taskFunc harmonytask.AddTaskFu SectorNumber int64 `db:"sector_number"` } - err := p.db.Select(ctx, &tasks, `SELECT sp_id, sector_number FROM sectors_snap_pipeline WHERE after_encode = TRUE AND after_prove = FALSE AND task_id_prove IS NULL`) + err := tx.Select(&tasks, `SELECT sp_id, sector_number FROM sectors_snap_pipeline WHERE after_encode = TRUE AND after_prove = FALSE AND task_id_prove IS NULL`) if err != nil { return false, xerrors.Errorf("getting tasks: %w", err) } diff --git a/tasks/snap/task_submit.go b/tasks/snap/task_submit.go index 2634c84c2..8243c6e80 100644 --- a/tasks/snap/task_submit.go +++ b/tasks/snap/task_submit.go @@ -469,7 +469,7 @@ func (s *SubmitTask) schedule(ctx context.Context, taskFunc harmonytask.AddTaskF SectorNumber int64 `db:"sector_number"` } - err := s.db.Select(ctx, &tasks, `SELECT sp_id, sector_number FROM sectors_snap_pipeline WHERE failed = FALSE + err := tx.Select(&tasks, `SELECT sp_id, sector_number FROM sectors_snap_pipeline WHERE failed = FALSE AND after_encode = TRUE AND after_prove = TRUE AND after_submit = FALSE diff --git a/tasks/unseal/task_unseal_decode.go b/tasks/unseal/task_unseal_decode.go index 2755644ae..40b0839a5 100644 --- a/tasks/unseal/task_unseal_decode.go +++ b/tasks/unseal/task_unseal_decode.go @@ -141,6 +141,8 @@ func (t *TaskUnsealDecode) Do(taskID harmonytask.TaskID, stillOwned func() bool) } } + // NOTE: Decode.. drops the sector key at the end + _, err = t.db.Exec(ctx, `UPDATE sectors_unseal_pipeline SET after_decode_sector = TRUE, task_id_decode_sector = NULL WHERE task_id_decode_sector = $1`, taskID) if err != nil { return false, xerrors.Errorf("updating task: %w", err)