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

feat: funds distribution #244

Merged
merged 4 commits into from
Dec 20, 2024
Merged

feat: funds distribution #244

merged 4 commits into from
Dec 20, 2024

Conversation

gluax
Copy link
Collaborator

@gluax gluax commented Dec 19, 2024

Motivation

Distribution of data request escrow funds.

Explanation of Changes

  • Iterate over provided data request ids in removal.
  • Iterate over each message for the data request.
    • It emits events for each message
    • It moves appropriate funds
  • If funds are leftover uses the refund, and refund message.
    • The reason there is a refund_type is so for events if we are doing a full refund we can give a reason. But most times it will be remainder payback if it exists.
  • Removes the Escrow from the map.

Testing

  • Test util changes better track executor funds.
  • Test that messages work and funds update as expected

Related PRs and Issues

Waiting for sedaprotocol/seda-common-rs#34 to be merged so we can update common to main.
Closes #243

@gluax gluax self-assigned this Dec 19, 2024
Copy link
Member

@Thomasvdam Thomasvdam left a comment

Choose a reason for hiding this comment

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

With our current design this won't work since we don't register a SEDA address for an executor node.

I think we should differentiate between data proxy payouts and executor payouts. Data proxy is a simple bank transfer, but an executor payout is an internal accounting change on the contract with the funds staying on the contract.
We discussed having moving the funds to the identities' staking pool, and anything that exceeds the minimum_to_stake amount goes straight to the to_withdraw pool. Given that we're starting with an allowlisted pool of executors we could opt to simplify this and always move it to one of them.

@gluax
Copy link
Collaborator Author

gluax commented Dec 19, 2024

With our current design this won't work since we don't register a SEDA address for an executor node.

I think we should differentiate between data proxy payouts and executor payouts. Data proxy is a simple bank transfer, but an executor payout is an internal accounting change on the contract with the funds staying on the contract. We discussed having moving the funds to the identities' staking pool, and anything that exceeds the minimum_to_stake amount goes straight to the to_withdraw pool. Given that we're starting with an allowlisted pool of executors we could opt to simplify this and always move it to one of them.

Oh I totally forgot about that it's been so long since we talked about it.

So need to add another DistributionMessage that is a executor reward that takes the identity and moves it to their staking pool up to the minimum amount to stake. The rest gets point into a new RewardEscrow the identity could withdraw from? So I need to add a new execute method to withdraw from that, and probably a query function to view your reward escrow?

I realized it could still use the DistributionSend Message, and we can match on the type attached, to see if it's executor reward and then do appropriate action instead of doing another message there. But I think a new message type is less error prone

@Thomasvdam
Copy link
Member

Right now an executor has stake and funds_ready_to_withdraw or something similar. I guess we can use those two fields for now and don't bother with a unstaking period.

Copy link
Member

Choose a reason for hiding this comment

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

Do we want to bump the version here too? Or not yet?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I forgot I'll do that now ^^

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done in #242.

Base automatically changed from feat/stake-dr-post-funds to main December 20, 2024 15:55
@gluax gluax force-pushed the feat/funds-distribution branch from 8aa052d to dd80661 Compare December 20, 2024 15:57

if !dr_escrow.amount.is_zero() {
bank_messages.push(BankMsg::Send {
to_address: dr_escrow.staker.to_string(),
Copy link
Member

Choose a reason for hiding this comment

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

This should be poster

Suggested change
to_address: dr_escrow.staker.to_string(),
to_address: dr_escrow.poster.to_string(),

@gluax gluax merged commit 34f308a into main Dec 20, 2024
2 checks passed
@gluax gluax deleted the feat/funds-distribution branch December 20, 2024 16:18
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.

✨ Distribution Messages
3 participants