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

Implement LQT reward distribution #5045

Merged
merged 13 commits into from
Feb 3, 2025

Conversation

cronokirby
Copy link
Contributor

Describe your changes

This integrates the recent work in the funding component, providing a full implementation of the reward calculation and distribution logic, running at the end of each epoch.

There are some unit and prop tests for some core tallying logic, but we'll need to do manual testing of the various RPCs an what not later.

Checklist before requesting a review

  • I have added guiding text to explain how a reviewer should test these changes.

  • If this code contains consensus-breaking changes, I have added the "consensus-breaking" label. Otherwise, I declare my belief that there are not consensus-breaking changes, for the following reason:

@cronokirby cronokirby added the consensus-breaking breaking change to execution of on-chain data label Feb 3, 2025
@cronokirby
Copy link
Contributor Author

cronokirby commented Feb 3, 2025

One design quirk I'm running into is that the current tallying logic can actually reward a voter for multiple votes in the same epoch, if they vote for different assets, using the same address. (Voting for the same asset twice results in overwriting your vote, under the current logic). Conceptually, this would require amending the commitment source for LQTs to have several tx ids.

Copy link
Member

@erwanor erwanor left a comment

Choose a reason for hiding this comment

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

Exciting, I did a first pass, will circle back to it later this morning

@cronokirby cronokirby requested a review from erwanor February 3, 2025 17:29
@cronokirby cronokirby changed the title Cronokirby/lqt integration Implement LQT reward distribution Feb 3, 2025
@erwanor
Copy link
Member

erwanor commented Feb 3, 2025

Posted in a comment reply in vscode, but this doesn't show up here:

That make sense to me, reaching for preallocs is a good instinct, and as is there is definitely massive allocator pressure. But I think this is a sign we shouldn't load the entire asset space in-memory to filter it, like discussed out of band.

Instead, we can do another pass so that:

  1. Every vote cast increments a total tally for the epoch, across assets. That's the total_voting_power for the epoch.
  2. We also maintain a sorted index of assets by voting power, that we update as we go
  3. To build the finalized gauge, we can prefix scan that index up to threshold_cutoff_pct.apply_to_amount(total_voting_power) (M entries)
  4. We have a consolidated asset list, and accumulate the total voting power for the gauge after cutoff —> we don’t care about the full index width anymore
  5. For each of the M assets, we can do a prefix scan for the top N votes entries (consolidated, one entry per beneficiary address)
  6. We accumulate the voting power of those top N entries to find asset_voting_power and use it with total_gauge_voting_power to find the gauge weight for the asset

In retrospect, I regret not adding plausible ranges for N and M to the ADR. I think we want to have 1 < N < 10000 and 1 < M < 25.

With that in mind, we will be able to get rid of the nested BTreeMap, and instead:

  1. Keep a SmallVec (or wtv.) of (asset::Id, Weight) (Finalized gauge)
  2. We don’t need to track of all the voters in memory, we can make a prefix scan with great locality to get the voters and mint opportunistically (mint queue

Thoughts?

Copy link
Member

@erwanor erwanor left a comment

Choose a reason for hiding this comment

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

Let's merge this and do an aggressive follow-up work so that:

  1. we can efficiently trim the assets below the cutoff without reading the whole index
  2. we only track the gauge weights and defer the heavy lifting to sorted prefix scans in rocksdb

I think the sketch above this comment achieves that

@cronokirby cronokirby merged commit 8cc540a into protocol/lqt_branch Feb 3, 2025
13 checks passed
@cronokirby cronokirby deleted the cronokirby/lqt-integration branch February 3, 2025 23:15
conorsch pushed a commit that referenced this pull request Feb 4, 2025
## Describe your changes

This integrates the recent work in the funding component, providing a
full implementation of the reward calculation and distribution logic,
running at the end of each epoch.

There are some unit and prop tests for some core tallying logic, but
we'll need to do manual testing of the various RPCs an what not later.

## Checklist before requesting a review

- [x] I have added guiding text to explain how a reviewer should test
these changes.

- [x] If this code contains consensus-breaking changes, I have added the
"consensus-breaking" label. Otherwise, I declare my belief that there
are not consensus-breaking changes, for the following reason:
conorsch pushed a commit that referenced this pull request Feb 5, 2025
## Describe your changes

This integrates the recent work in the funding component, providing a
full implementation of the reward calculation and distribution logic,
running at the end of each epoch.

There are some unit and prop tests for some core tallying logic, but
we'll need to do manual testing of the various RPCs an what not later.

## Checklist before requesting a review

- [x] I have added guiding text to explain how a reviewer should test
these changes.

- [x] If this code contains consensus-breaking changes, I have added the
"consensus-breaking" label. Otherwise, I declare my belief that there
are not consensus-breaking changes, for the following reason:
conorsch pushed a commit that referenced this pull request Feb 5, 2025
## Describe your changes

This integrates the recent work in the funding component, providing a
full implementation of the reward calculation and distribution logic,
running at the end of each epoch.

There are some unit and prop tests for some core tallying logic, but
we'll need to do manual testing of the various RPCs an what not later.

## Checklist before requesting a review

- [x] I have added guiding text to explain how a reviewer should test
these changes.

- [x] If this code contains consensus-breaking changes, I have added the
"consensus-breaking" label. Otherwise, I declare my belief that there
are not consensus-breaking changes, for the following reason:
conorsch pushed a commit that referenced this pull request Feb 14, 2025
## Describe your changes

This integrates the recent work in the funding component, providing a
full implementation of the reward calculation and distribution logic,
running at the end of each epoch.

There are some unit and prop tests for some core tallying logic, but
we'll need to do manual testing of the various RPCs an what not later.

## Checklist before requesting a review

- [x] I have added guiding text to explain how a reviewer should test
these changes.

- [x] If this code contains consensus-breaking changes, I have added the
"consensus-breaking" label. Otherwise, I declare my belief that there
are not consensus-breaking changes, for the following reason:
conorsch pushed a commit that referenced this pull request Feb 21, 2025
## Describe your changes

This integrates the recent work in the funding component, providing a
full implementation of the reward calculation and distribution logic,
running at the end of each epoch.

There are some unit and prop tests for some core tallying logic, but
we'll need to do manual testing of the various RPCs an what not later.

## Checklist before requesting a review

- [x] I have added guiding text to explain how a reviewer should test
these changes.

- [x] If this code contains consensus-breaking changes, I have added the
"consensus-breaking" label. Otherwise, I declare my belief that there
are not consensus-breaking changes, for the following reason:
cronokirby added a commit that referenced this pull request Feb 27, 2025
## Describe your changes

This integrates the recent work in the funding component, providing a
full implementation of the reward calculation and distribution logic,
running at the end of each epoch.

There are some unit and prop tests for some core tallying logic, but
we'll need to do manual testing of the various RPCs an what not later.

## Checklist before requesting a review

- [x] I have added guiding text to explain how a reviewer should test
these changes.

- [x] If this code contains consensus-breaking changes, I have added the
"consensus-breaking" label. Otherwise, I declare my belief that there
are not consensus-breaking changes, for the following reason:
conorsch pushed a commit that referenced this pull request Mar 18, 2025
## Describe your changes

This integrates the recent work in the funding component, providing a
full implementation of the reward calculation and distribution logic,
running at the end of each epoch.

There are some unit and prop tests for some core tallying logic, but
we'll need to do manual testing of the various RPCs an what not later.

## Checklist before requesting a review

- [x] I have added guiding text to explain how a reviewer should test
these changes.

- [x] If this code contains consensus-breaking changes, I have added the
"consensus-breaking" label. Otherwise, I declare my belief that there
are not consensus-breaking changes, for the following reason:
conorsch pushed a commit that referenced this pull request Mar 24, 2025
## Describe your changes

This integrates the recent work in the funding component, providing a
full implementation of the reward calculation and distribution logic,
running at the end of each epoch.

There are some unit and prop tests for some core tallying logic, but
we'll need to do manual testing of the various RPCs an what not later.

## Checklist before requesting a review

- [x] I have added guiding text to explain how a reviewer should test
these changes.

- [x] If this code contains consensus-breaking changes, I have added the
"consensus-breaking" label. Otherwise, I declare my belief that there
are not consensus-breaking changes, for the following reason:
cronokirby added a commit that referenced this pull request Mar 28, 2025
## Describe your changes

This integrates the recent work in the funding component, providing a
full implementation of the reward calculation and distribution logic,
running at the end of each epoch.

There are some unit and prop tests for some core tallying logic, but
we'll need to do manual testing of the various RPCs an what not later.

## Checklist before requesting a review

- [x] I have added guiding text to explain how a reviewer should test
these changes.

- [x] If this code contains consensus-breaking changes, I have added the
"consensus-breaking" label. Otherwise, I declare my belief that there
are not consensus-breaking changes, for the following reason:
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
consensus-breaking breaking change to execution of on-chain data
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants