Implement account rewards #335
Conversation
lassemand
changed the title
| SpecialTransactionOutcome::BlockAccrueReward { | ||
| .. | ||
| } | ||
| | SpecialTransactionOutcome::PaydayPoolReward { | ||
| .. | ||
| } => Vec::new(), |
There was a problem hiding this comment.
I am surprised these are not implemented as these also include rewards for accounts
There was a problem hiding this comment.
Lets implement that
There was a problem hiding this comment.
As this deviates from the existing flow I would like to wait with this. I added a TODO, and will do that after I have validated correctness between the two instances of CCDScan.
There was a problem hiding this comment.
Pretty sure these outcomes were introduced in a later protocol version, so this might be why the code for handling it is scattered in the old backend.
Co-authored-by: Emil Holm Gjørup <eg@concordium.com>
…ordium/concordium-scan into lma/CCD-74/implement-accountrewards
| amount, | ||
| block_height | ||
| FROM account_statements | ||
| WHERE entry_type::TEXT IN ( |
There was a problem hiding this comment.
Don't think that we need to cast it to TEXT here, comparing enums values seems better.
| WHERE entry_type::TEXT IN ( | |
| WHERE entry_type IN ( |
|
|
||
| CREATE INDEX account_statements_entry_type_idx ON account_statements (entry_type); | ||
|
|
||
| CREATE VIEW account_rewards AS |
There was a problem hiding this comment.
Not sure how this performs, since the query we need is the rewards for some account_index and this view is first filtering rewards across every account, and then we filter the ones related to the account (scanning potentially). Maybe postgres is smart enough to rearrange to avoid scanning, but a more direct approach would be to have the index:
account_statements (account_index, entry_type)
And then query directly:
SELECT ... FROM account_statements WHERE account_index = $1 AND entry_type IN (...)Which I expect to scale better
There was a problem hiding this comment.
A view is essentially a stored query that you can treat almost like a table. So performance will be the same.
Even if performance would be an issue this is to be used on a single account. This is by no means a bottleneck of anything, lets not waste time on pseudo optimisations.
| let mut account_statements = sqlx::query_as!( | ||
| AccountStatementEntry, | ||
| r#" | ||
| SELECT |
There was a problem hiding this comment.
I think we are missing the subquery for handling the last query parameter, currently it seems that it will behave identically to providing first as we are not ordering a subquery according to query.desc (which is true when last is provided, the bad naming of query.desc is on me)
| config.reward_connection_limit, | ||
| )?; | ||
| let mut rewards = sqlx::query_as!( | ||
| AccountReward, |
There was a problem hiding this comment.
Similar to above, I think we are missing handling of last query parameter
| SpecialTransactionOutcome::BlockAccrueReward { | ||
| .. | ||
| } | ||
| | SpecialTransactionOutcome::PaydayPoolReward { | ||
| .. | ||
| } => Vec::new(), |
There was a problem hiding this comment.
Pretty sure these outcomes were introduced in a later protocol version, so this might be why the code for handling it is scattered in the old backend.
Tasks:
https://linear.app/concordium/issue/CCD-79/implement-blockspecial-events
https://linear.app/concordium/issue/CCD-74/implement-accountrewards
https://linear.app/concordium/issue/CCD-85/implement-accountaccount-statement
Purpose
Index special events
Query account statements
Query rewards
We are not including the account statements generated from transactions as part of this PR. Leaving that for the next PR to limit scope of this PR.
Checklist
hard-to-understand areas.