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: add total reward for epoch #204

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ALTER TABLE pos_rewards DROP COLUMN epoch;
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
-- Alters the pos_rewards table to add a fourth column:
-- epoch: INTEGER, representing the epoch number
-- Populate existing records with epoch = 0
ALTER TABLE pos_rewards ADD COLUMN epoch INTEGER NOT NULL DEFAULT 0;
-- Now we can safely drop the default
ALTER TABLE pos_rewards ALTER COLUMN epoch DROP DEFAULT;
Copy link
Contributor

Choose a reason for hiding this comment

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

While testing this, I ran into a problem where the pos_rewards table didn't have an updated UNIQUE constraint which incorporates the epoch column.

I pushed a commit that fixes that: 725f9c9

Feel free to add it to your branch also.

Copy link
Author

Choose a reason for hiding this comment

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

Updated that here:

1244bab

17 changes: 7 additions & 10 deletions orm/src/pos_rewards.rs
Original file line number Diff line number Diff line change
@@ -1,29 +1,26 @@
use std::str::FromStr;

use bigdecimal::BigDecimal;
use diesel::{Insertable, Queryable, Selectable};
use shared::rewards::Reward;

use crate::schema::pos_rewards;
use shared::rewards::Reward;

#[derive(Insertable, Clone, Queryable, Selectable)]
#[derive(Insertable, Queryable, Selectable, Clone)]
#[diesel(table_name = pos_rewards)]
#[diesel(check_for_backend(diesel::pg::Pg))]
pub struct PosRewardInsertDb {
pub owner: String,
pub validator_id: i32,
pub raw_amount: BigDecimal,
pub epoch: i32,
}

pub type PoSRewardDb = PosRewardInsertDb;

impl PosRewardInsertDb {
pub fn from_reward(reward: Reward, validator_id: i32) -> Self {
Self {
pub fn from_reward(reward: Reward, validator_id: i32, epoch: i32) -> Self {
PosRewardInsertDb {
owner: reward.delegation_pair.delegator_address.to_string(),
raw_amount: BigDecimal::from_str(&reward.amount.to_string())
.expect("Invalid amount"),
validator_id,
raw_amount: BigDecimal::from(reward.amount),
Copy link
Contributor

Choose a reason for hiding this comment

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

Note - this is assigning to raw_amount the decimal value of nam (i.e. 12.34567 not 1234567 unam) -- which is different than the previous behavior of the rewards crawler. I noticed it in my testing because the raw_amount column has type numeric(78,0) which only stores integer values, and the reward amounts were being truncated.

Copy link
Author

Choose a reason for hiding this comment

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

0957a4a

Fixed here with:

            raw_amount: BigDecimal::from_str(&reward.amount.to_string()).unwrap(),

epoch,
}
}
}
67 changes: 12 additions & 55 deletions orm/src/schema.rs
Original file line number Diff line number Diff line change
@@ -1,91 +1,47 @@
// @generated automatically by Diesel CLI.

pub mod sql_types {
#[derive(
diesel::query_builder::QueryId,
std::fmt::Debug,
diesel::sql_types::SqlType,
)]
#[derive(diesel::query_builder::QueryId, std::fmt::Debug, diesel::sql_types::SqlType)]
#[diesel(postgres_type(name = "crawler_name"))]
pub struct CrawlerName;

#[derive(
diesel::query_builder::QueryId,
std::fmt::Debug,
diesel::sql_types::SqlType,
)]
#[derive(diesel::query_builder::QueryId, std::fmt::Debug, diesel::sql_types::SqlType)]
#[diesel(postgres_type(name = "governance_kind"))]
pub struct GovernanceKind;

#[derive(
diesel::query_builder::QueryId,
std::fmt::Debug,
diesel::sql_types::SqlType,
)]
#[derive(diesel::query_builder::QueryId, std::fmt::Debug, diesel::sql_types::SqlType)]
#[diesel(postgres_type(name = "governance_result"))]
pub struct GovernanceResult;

#[derive(
diesel::query_builder::QueryId,
std::fmt::Debug,
diesel::sql_types::SqlType,
)]
#[derive(diesel::query_builder::QueryId, std::fmt::Debug, diesel::sql_types::SqlType)]
#[diesel(postgres_type(name = "governance_tally_type"))]
pub struct GovernanceTallyType;

#[derive(
diesel::query_builder::QueryId,
std::fmt::Debug,
diesel::sql_types::SqlType,
)]
#[derive(diesel::query_builder::QueryId, std::fmt::Debug, diesel::sql_types::SqlType)]
#[diesel(postgres_type(name = "payment_kind"))]
pub struct PaymentKind;

#[derive(
diesel::query_builder::QueryId,
std::fmt::Debug,
diesel::sql_types::SqlType,
)]
#[derive(diesel::query_builder::QueryId, std::fmt::Debug, diesel::sql_types::SqlType)]
#[diesel(postgres_type(name = "payment_recurrence"))]
pub struct PaymentRecurrence;

#[derive(
diesel::query_builder::QueryId,
std::fmt::Debug,
diesel::sql_types::SqlType,
)]
#[derive(diesel::query_builder::QueryId, std::fmt::Debug, diesel::sql_types::SqlType)]
#[diesel(postgres_type(name = "token_type"))]
pub struct TokenType;

#[derive(
diesel::query_builder::QueryId,
std::fmt::Debug,
diesel::sql_types::SqlType,
)]
#[derive(diesel::query_builder::QueryId, std::fmt::Debug, diesel::sql_types::SqlType)]
#[diesel(postgres_type(name = "transaction_kind"))]
pub struct TransactionKind;

#[derive(
diesel::query_builder::QueryId,
std::fmt::Debug,
diesel::sql_types::SqlType,
)]
#[derive(diesel::query_builder::QueryId, std::fmt::Debug, diesel::sql_types::SqlType)]
#[diesel(postgres_type(name = "transaction_result"))]
pub struct TransactionResult;

#[derive(
diesel::query_builder::QueryId,
std::fmt::Debug,
diesel::sql_types::SqlType,
)]
#[derive(diesel::query_builder::QueryId, std::fmt::Debug, diesel::sql_types::SqlType)]
#[diesel(postgres_type(name = "validator_state"))]
pub struct ValidatorState;

#[derive(
diesel::query_builder::QueryId,
std::fmt::Debug,
diesel::sql_types::SqlType,
)]
#[derive(diesel::query_builder::QueryId, std::fmt::Debug, diesel::sql_types::SqlType)]
#[diesel(postgres_type(name = "vote_kind"))]
pub struct VoteKind;
}
Expand Down Expand Up @@ -227,6 +183,7 @@ diesel::table! {
owner -> Varchar,
validator_id -> Int4,
raw_amount -> Numeric,
epoch -> Int4,
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @neocybereth -- I have another suggestion here, it looks like you've reformatted a bunch of things in this file that aren't really part of this branch. It would be best to keep your changes minimal and not include these formatting changes.

I think this line is the only change that you should actually have in this file.

Copy link
Contributor

Choose a reason for hiding this comment

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

I just pushed a commit that restores that formatting here, and also has one other small refactor in the rewards/src/services/namada.rs to reuse the existing function for querying current epoch instead of copying it: 2d0a5ac

Feel free to incorporate it into your branch here

Copy link
Author

Choose a reason for hiding this comment

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

alright I've integrated both of those 👍

}
}

Expand Down
8 changes: 6 additions & 2 deletions rewards/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,9 @@ async fn crawling_fn(

tracing::info!("Starting to update proposals...");

let current_epoch = namada_service::get_current_epoch(&client)
.await
.into_rpc_error()?;
// TODO: change this by querying all the pairs in the database
let delegations_pairs = namada_service::query_delegation_pairs(&client)
.await
Expand All @@ -100,18 +103,19 @@ async fn crawling_fn(

conn.interact(move |conn| {
conn.build_transaction().read_write().run(
|transaction_conn: &mut diesel::prelude::PgConnection| {
|transaction_conn: &mut diesel::pg::PgConnection| {
repository::pos_rewards::upsert_rewards(
transaction_conn,
non_zero_rewards,
current_epoch as i32,
)?;

repository::crawler_state::upsert_crawler_state(
transaction_conn,
(CrawlerName::Rewards, crawler_state).into(),
)?;

anyhow::Ok(())
Ok(())
},
)
})
Expand Down
13 changes: 6 additions & 7 deletions rewards/src/repository/pos_rewards.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,30 +7,29 @@ use shared::rewards::Reward;
pub fn upsert_rewards(
transaction_conn: &mut PgConnection,
rewards: Vec<Reward>,
epoch: i32, // Add an epoch parameter
) -> anyhow::Result<()> {
diesel::insert_into(pos_rewards::table)
.values::<Vec<PosRewardInsertDb>>(
rewards
.into_iter()
.map(|reward| {
let validator_id: i32 = validators::table
.filter(
validators::namada_address.eq(&reward
.delegation_pair
.validator_address
.to_string()),
)
.filter(validators::namada_address.eq(
&reward.delegation_pair.validator_address.to_string(),
))
.select(validators::id)
.first(transaction_conn)
.expect("Failed to get validator");

PosRewardInsertDb::from_reward(reward, validator_id)
PosRewardInsertDb::from_reward(reward, validator_id, epoch)
})
.collect::<Vec<_>>(),
)
.on_conflict((
pos_rewards::columns::owner,
pos_rewards::columns::validator_id,
pos_rewards::columns::epoch, // Add epoch to conflict target
))
.do_update()
.set(
Expand Down
4 changes: 4 additions & 0 deletions rewards/src/services/namada.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ pub async fn query_rewards(
client: &HttpClient,
delegation_pairs: HashSet<DelegationPair>,
) -> anyhow::Result<Vec<Reward>> {
let epoch = rpc::query_epoch(client)
.await
.context("Failed to query Namada's current epoch")?;
Ok(futures::stream::iter(delegation_pairs)
.filter_map(|delegation| async move {
tracing::info!(
Expand Down Expand Up @@ -64,6 +67,7 @@ pub async fn query_rewards(
Some(Reward {
delegation_pair: delegation,
amount: Amount::from(reward),
epoch: epoch.0 as i32,
})
})
.map(futures::future::ready)
Expand Down
3 changes: 2 additions & 1 deletion seeder/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,8 @@ async fn main() -> anyhow::Result<(), MainError> {
.into_iter()
.map(|reward| {
let validator_id = reward.delegation_pair.validator_address.to_string().parse::<i32>().unwrap();
PosRewardInsertDb::from_reward(reward, validator_id)
let epoch = reward.epoch;
PosRewardInsertDb::from_reward(reward, validator_id, epoch)
})
.collect::<Vec<_>>(),
)
Expand Down
46 changes: 46 additions & 0 deletions shared/src/balance.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use std::fmt::Display;
use std::str::FromStr;

use bigdecimal::BigDecimal;
use fake::Fake;
Expand Down Expand Up @@ -28,6 +29,13 @@ impl From<BigDecimal> for Amount {
}
}

impl From<Amount> for BigDecimal {
fn from(amount: Amount) -> BigDecimal {
BigDecimal::from_str(&amount.0.to_string_native())
.expect("Invalid amount")
}
}

impl Display for Amount {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "{}", self.0)
Expand Down Expand Up @@ -114,3 +122,41 @@ impl Balance {
}
}
}

#[cfg(test)]
mod tests {
use crate::balance::{Amount, NamadaAmount};
use bigdecimal::BigDecimal;
use namada_sdk::token::NATIVE_MAX_DECIMAL_PLACES;
use std::str::FromStr;
#[test]
fn test_bigquery_amount_round_trip_integer() {
let bigdecimal =
BigDecimal::from(100).with_scale(NATIVE_MAX_DECIMAL_PLACES.into());
let amount = Amount::from(bigdecimal.clone());
let bigdecimal_from_amount = BigDecimal::from(amount.clone());
assert_eq!(amount, Amount::from(bigdecimal_from_amount.clone()));
assert_eq!(bigdecimal, bigdecimal_from_amount);
}
#[test]
fn test_bigquery_amount_round_trip_decimal() {
let bigdecimal = BigDecimal::from_str("100.123456").unwrap();
let amount = Amount::from(bigdecimal.clone());
let bigdecimal_from_amount = BigDecimal::from(amount.clone());
assert_eq!(amount, Amount::from(bigdecimal_from_amount.clone()));
assert_eq!(bigdecimal, bigdecimal_from_amount);
}
#[test]
fn test_amount_same_as_namada_amount_integer() {
let amount = Amount::from(BigDecimal::from(100));
let namada_amount = NamadaAmount::from_u64(100);
assert_eq!(amount.0, namada_amount);
}
#[test]
fn test_amount_same_as_namada_amount_decimal() {
let amount = Amount::from(BigDecimal::from_str("100.123").unwrap());
let namada_amount = NamadaAmount::from_string_precise("100.123")
.expect("Invalid amount");
assert_eq!(amount.0, namada_amount);
}
}
2 changes: 2 additions & 0 deletions shared/src/rewards.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use crate::utils::DelegationPair;
pub struct Reward {
pub delegation_pair: DelegationPair,
pub amount: Amount,
pub epoch: i32,
}

impl Reward {
Expand All @@ -19,6 +20,7 @@ impl Reward {
delegator_address: Id::Account(delegator_address.to_string()),
},
amount: Amount::fake(),
epoch: 0,
}
}
}