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

rfc: Add deal parameters #763

Merged
merged 6 commits into from
Feb 20, 2025
Merged

rfc: Add deal parameters #763

merged 6 commits into from
Feb 20, 2025

Conversation

aidan46
Copy link
Contributor

@aidan46 aidan46 commented Feb 18, 2025

Description

This PR adds an RFC for adding deal parameters that the storage provider can set. Allowing use-case like automatic deal making and auction houses.

Closes #760

@aidan46 aidan46 added the rfc Request for Comments (RFC) label Feb 18, 2025
@aidan46 aidan46 added this to the Phase 3 milestone Feb 18, 2025
@aidan46 aidan46 self-assigned this Feb 18, 2025
@aidan46 aidan46 marked this pull request as ready for review February 18, 2025 15:48
Copy link
Contributor

@th7nder th7nder left a comment

Choose a reason for hiding this comment

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

If we had to go with it as a MVP, I'd approve it. Left some comments to consider, but overall looks good. I don't know, when I was thinking about it the first time it felt more complex, maybe I was overthinking.

I think that the choice that SPs are setting those parameters by themselves is simplifying this work a lot. We'd need to suggest 'some prices for them' as a default for starters, to encourage SP to onboard, but I think from the technical standpoint it'll work.

@aidan46 aidan46 requested a review from th7nder February 20, 2025 10:41
@aidan46 aidan46 force-pushed the rfc/760-deal-parameters branch from e953285 to 30747b1 Compare February 20, 2025 10:49
@aidan46 aidan46 added the ready for review Review is needed label Feb 20, 2025
jmg-duarte
jmg-duarte previously approved these changes Feb 20, 2025
Copy link
Collaborator

@jmg-duarte jmg-duarte left a comment

Choose a reason for hiding this comment

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

For a PoC, LGTM

@aidan46 aidan46 enabled auto-merge (squash) February 20, 2025 13:29
Copy link
Collaborator

@jmg-duarte jmg-duarte left a comment

Choose a reason for hiding this comment

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

LGTM we just need to finish discussing the parameter removal/modification

@aidan46 aidan46 merged commit 52bea45 into develop Feb 20, 2025
6 checks passed
@aidan46 aidan46 deleted the rfc/760-deal-parameters branch February 20, 2025 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready for review Review is needed rfc Request for Comments (RFC)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rfc: Deal parameters
5 participants