Skip to content

[in progress] Curio Snark Marketplace #484

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

Draft
wants to merge 54 commits into
base: main
Choose a base branch
from
Draft

[in progress] Curio Snark Marketplace #484

wants to merge 54 commits into from

Conversation

magik6k
Copy link
Collaborator

@magik6k magik6k commented Apr 17, 2025

Not yet in the final form, some data structures might still change, especially some details around payments.

Tasks are mostly done other than the Request tasks

Snap/WDPoSt support also todo

@magik6k magik6k marked this pull request as draft April 17, 2025 13:45
Copy link
Contributor

@LexLuthr LexLuthr left a comment

Choose a reason for hiding this comment

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

Looks mostly good. We need solid documentation around this thing or dev side debugging would be nightmare without you.

Comment on lines +45 to +47
ticker := time.NewTicker(10 * time.Second)
go func() {
for range ticker.C {
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't this goroutine keep running forever?

SET compute_task_id = $1
WHERE service_id = $2
RETURNING service_id
`, taskID, serviceID).Scan(&serviceID)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are scanning the serviceID here? Did we plan to match with the old one?


// If we don't have a client request yet, create one
if errors.Is(err, pgx.ErrNoRows) {
sectorInfo, err := t.getSectorInfo(ctx, taskID)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't we just pass this from caller instead of calculating again?

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add some comments to what these individual tasks are doing?

Copy link
Contributor

Choose a reason for hiding this comment

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

Since this is Curio command, we need to add translation.

@@ -385,6 +385,22 @@ type CurioSubsystemsConfig struct {
// for the sector. Please ensure that TreeD and TreeRC task are enabled and relevant resources are available before
// enabling this option. (Default: false)
BindSDRTreeToNode bool

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add default values to comments?

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