From c896a82a9d03a2f01594476ef81c490fe9754737 Mon Sep 17 00:00:00 2001 From: Michael Jeffrey Date: Wed, 24 Apr 2024 10:05:20 -0700 Subject: [PATCH 1/8] Refactor to use BoostedHexes methods Making the internal member hexes private will make it easier to change the implementation when device type is introduced. --- mobile_config/src/boosted_hex_info.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/mobile_config/src/boosted_hex_info.rs b/mobile_config/src/boosted_hex_info.rs index 0e31bb548..20d709f60 100644 --- a/mobile_config/src/boosted_hex_info.rs +++ b/mobile_config/src/boosted_hex_info.rs @@ -225,6 +225,22 @@ impl BoostedHexes { pub fn insert(&mut self, info: BoostedHexInfo) { self.hexes.entry(info.location).or_default().push(info); } + + pub fn count(&self) -> usize { + self.hexes.len() + } + + pub fn iter_hexes(&self) -> impl Iterator { + self.hexes.values() + } + + pub fn get(&self, location: &Cell) -> Option<&BoostedHexInfo> { + self.hexes.get(location) + } + + pub fn insert(&mut self, info: BoostedHexInfo) { + self.hexes.insert(info.location, info); + } } pub(crate) mod db { From 2b558ca8d18ac99c4ee21690ddeb49072e2bc9a0 Mon Sep 17 00:00:00 2001 From: Michael Jeffrey Date: Wed, 24 Apr 2024 16:46:01 -0700 Subject: [PATCH 2/8] Add device type to boosted hex info --- boost_manager/tests/integrations/watcher_tests.rs | 5 ++++- mobile_verifier/tests/integrations/hex_boosting.rs | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/boost_manager/tests/integrations/watcher_tests.rs b/boost_manager/tests/integrations/watcher_tests.rs index a403efab7..78d180eca 100644 --- a/boost_manager/tests/integrations/watcher_tests.rs +++ b/boost_manager/tests/integrations/watcher_tests.rs @@ -2,7 +2,10 @@ use crate::common::{self, MockFileSinkReceiver, MockHexBoostingClient}; use boost_manager::watcher::{self, Watcher}; use chrono::{Duration as ChronoDuration, Duration, Utc}; use helium_proto::BoostedHexInfoV1 as BoostedHexInfoProto; -use mobile_config::boosted_hex_info::{BoostedHexDeviceType, BoostedHexInfo}; +use mobile_config::{ + boosted_hex_info::{BoostedHexDeviceType, BoostedHexInfo, BoostedHexInfoStream}, + client::{hex_boosting_client::HexBoostingInfoResolver, ClientError}, +}; use solana_sdk::pubkey::Pubkey; use sqlx::PgPool; use std::{num::NonZeroU32, str::FromStr}; diff --git a/mobile_verifier/tests/integrations/hex_boosting.rs b/mobile_verifier/tests/integrations/hex_boosting.rs index 3ec1d584e..856a14bd9 100644 --- a/mobile_verifier/tests/integrations/hex_boosting.rs +++ b/mobile_verifier/tests/integrations/hex_boosting.rs @@ -11,7 +11,10 @@ use helium_proto::services::poc_mobile::{ UnallocatedReward, }; use hextree::Cell; -use mobile_config::boosted_hex_info::{BoostedHexDeviceType, BoostedHexInfo}; +use mobile_config::{ + boosted_hex_info::{BoostedHexDeviceType, BoostedHexInfo, BoostedHexInfoStream}, + client::{hex_boosting_client::HexBoostingInfoResolver, ClientError}, +}; use mobile_verifier::{ cell_type::CellType, coverage::{set_oracle_boosting_assignments, CoverageObject, UnassignedHex}, From 8091ceb88815e908e5b71a25618314f22efe2fa2 Mon Sep 17 00:00:00 2001 From: Michael Jeffrey Date: Wed, 1 May 2024 11:57:29 -0700 Subject: [PATCH 3/8] refactor metadata_db tests to make test clearer also makes it easier to add new tests --- mobile_config/src/boosted_hex_info.rs | 304 +++++++++++++++----------- 1 file changed, 175 insertions(+), 129 deletions(-) diff --git a/mobile_config/src/boosted_hex_info.rs b/mobile_config/src/boosted_hex_info.rs index 20d709f60..751f05b81 100644 --- a/mobile_config/src/boosted_hex_info.rs +++ b/mobile_config/src/boosted_hex_info.rs @@ -541,150 +541,196 @@ mod tests { Ok(()) } - #[sqlx::test] - #[ignore = "for manual metadata db testing"] - async fn parse_boosted_hex_info_from_database(pool: PgPool) -> anyhow::Result<()> { - let boost_config_address = Pubkey::new_unique(); - let now = Utc::now(); + fn parse_dt(dt: &str) -> DateTime { + NaiveDateTime::parse_from_str(dt, "%Y-%m-%d %H:%M:%S") + .expect("unable_to_parse") + .and_utc() + } - // NOTE(mj): Table creation taken from a dump of the metadata db. - // device_type was added to boosted_hexes as a jsonb field because the - // mobile_hotspot_infos table has a device_type column that maps to an - // enum, and it is a nullable jsonb column. - const CREATE_BOOSTED_HEXES_TABLE: &str = r#" - CREATE TABLE - boosted_hexes ( - address character varying(255) NOT NULL PRIMARY KEY, - boost_config character varying(255) NULL, - location numeric NULL, - start_ts numeric NULL, - reserved numeric[] NULL, - bump_seed integer NULL, - boosts_by_period bytea NULL, - version integer NULL, - refreshed_at timestamp with time zone NULL, - created_at timestamp with time zone NOT NULL, - device_type jsonb - ) - "#; + mod metadata_db { + /// Table creation was taken from a dump of the metadata db. + /// `device_type` was added to boosted_hexes as a jsonb field because the + /// mobile_hotspot_infos table has a device_type column that maps to an + /// enum, and it is a nullable jsonb column. + /// + /// When the `boosted_hexes` or `boost_configs` tables from the metadata_db + /// are updated, these tests will not break until the new table formats are + /// put into `create_tables()`. + use super::*; + + #[sqlx::test] + async fn parse_boosted_hex_info_from_database(pool: PgPool) -> anyhow::Result<()> { + let boost_config_address = Pubkey::new_unique(); + let now = Utc::now(); + + create_tables(&pool).await?; + insert_boost_config(&pool, &boost_config_address, now).await?; + + let device_types = vec![ + None, // legacy boosted hex with NULL device_type + Some(serde_json::json!("cbrsIndoor")), + Some(serde_json::json!("cbrsOutdoor")), + Some(serde_json::json!("wifiIndoor")), + Some(serde_json::json!("wifiOutdoor")), + // Some(serde_json::json!(null)) // this is different from None, and will/should break parsing as it's not part of the enum + ]; + for device_type in device_types { + insert_boosted_hex(&pool, &boost_config_address, now, device_type, Some(now)) + .await?; + } + + assert_eq!( + 5, + boosted_hexes_count(&pool).await?, + "there should be 1 of each type of boosted hex" + ); + assert_eq!( + 5, + streamed_hexes_count(&pool).await?, + "not all rows were able to parse" + ); + + Ok(()) + } - const CREATE_BOOST_CONFIG_TABLE: &str = r#" - CREATE TABLE - boost_configs ( - address character varying(255) NOT NULL PRIMARY KEY, - price_oracle character varying(255) NULL, - payment_mint character varying(255) NULL, - sub_dao character varying(255) NULL, - rent_reclaim_authority character varying(255) NULL, - boost_price numeric NULL, - period_length integer NULL, - minimum_periods integer NULL, - bump_seed integer NULL, - start_authority character varying(255) NULL, - refreshed_at timestamp with time zone NULL, - created_at timestamp with time zone NOT NULL - ) - "#; + async fn create_tables(pool: &PgPool) -> anyhow::Result<()> { + const CREATE_BOOSTED_HEXES_TABLE: &str = r#" + CREATE TABLE + boosted_hexes ( + address character varying(255) NOT NULL PRIMARY KEY, + boost_config character varying(255) NULL, + location numeric NULL, + start_ts numeric NULL, + reserved numeric[] NULL, + bump_seed integer NULL, + boosts_by_period bytea NULL, + version integer NULL, + refreshed_at timestamp with time zone NULL, + created_at timestamp with time zone NOT NULL, + device_type jsonb + ) + "#; + + const CREATE_BOOST_CONFIG_TABLE: &str = r#" + CREATE TABLE + boost_configs ( + address character varying(255) NOT NULL PRIMARY KEY, + price_oracle character varying(255) NULL, + payment_mint character varying(255) NULL, + sub_dao character varying(255) NULL, + rent_reclaim_authority character varying(255) NULL, + boost_price numeric NULL, + period_length integer NULL, + minimum_periods integer NULL, + bump_seed integer NULL, + start_authority character varying(255) NULL, + refreshed_at timestamp with time zone NULL, + created_at timestamp with time zone NOT NULL + ) + "#; + + sqlx::query(CREATE_BOOSTED_HEXES_TABLE) + .execute(pool) + .await?; + sqlx::query(CREATE_BOOST_CONFIG_TABLE).execute(pool).await?; + Ok(()) + } - sqlx::query(CREATE_BOOSTED_HEXES_TABLE) - .execute(&pool) - .await?; - sqlx::query(CREATE_BOOST_CONFIG_TABLE) - .execute(&pool) - .await?; + async fn insert_boost_config( + pool: &PgPool, + boost_config_address: &Pubkey, + created_at: DateTime, + ) -> anyhow::Result<()> { + const INSERT_BOOST_CONFIG: &str = r#" + INSERT INTO boost_configs ( + "boost_price", "bump_seed", "minimum_periods", "period_length", "refreshed_at", + + -- pubkeys + "price_oracle", + "payment_mint", + "rent_reclaim_authority", + "start_authority", + "sub_dao", + + -- our values + "address", + "created_at" + ) + VALUES ( + '5000', 250, 6, 2592000, '2024-03-12 21:13:52.692+00', - const INSERT_BOOST_CONFIG: &str = r#" - INSERT INTO boost_configs ( - "boost_price", "bump_seed", "minimum_periods", "period_length", "refreshed_at", - - -- pubkeys - "price_oracle", - "payment_mint", - "rent_reclaim_authority", - "start_authority", - "sub_dao", - - -- our values - "address", - "created_at" - ) - VALUES ( - '5000', 250, 6, 2592000, '2024-03-12 21:13:52.692+00', - - $1, $2, $3, $4, $5, $6, $7 - ) - "#; + $1, $2, $3, $4, $5, $6, $7 + ) + "#; + + sqlx::query(INSERT_BOOST_CONFIG) + .bind(Pubkey::new_unique().to_string()) // price_oracle + .bind(Pubkey::new_unique().to_string()) // payment_mint + .bind(Pubkey::new_unique().to_string()) // rent_reclaim_authority + .bind(Pubkey::new_unique().to_string()) // start_authority + .bind(Pubkey::new_unique().to_string()) // sub_dao + // -- + .bind(boost_config_address.to_string()) // address + .bind(created_at) // created_at + .execute(pool) + .await?; - const INSERT_BOOSTED_HEX: &str = r#" - INSERT INTO boosted_hexes ( - "boosts_by_period", "bump_seed", "location", "refreshed_at", "reserved", "start_ts", "version", - - -- our values - "address", - "boost_config", - "created_at", - "device_type" - ) - VALUES ( - 'ZGRkZGRk', 1, '631798453297853439', '2024-03-12 21:13:53.773+00', '{0,0,0,0,0,0,0,0}', '1708304400', 1, - - $1, $2, $3, $4 - ) - "#; + Ok(()) + } - // Insert boost config that boosted hexes will point to. - sqlx::query(INSERT_BOOST_CONFIG) - .bind(Pubkey::new_unique().to_string()) // price_oracle - .bind(Pubkey::new_unique().to_string()) // payment_mint - .bind(Pubkey::new_unique().to_string()) // rent_reclaim_authority - .bind(Pubkey::new_unique().to_string()) // start_authority - .bind(Pubkey::new_unique().to_string()) // sub_dao - // -- - .bind(boost_config_address.to_string()) // address - .bind(now) // created_at - .execute(&pool) - .await?; + async fn insert_boosted_hex( + pool: &PgPool, + boost_config_address: &Pubkey, + created_at: DateTime, + device_type: Option, + start_ts: Option>, + ) -> anyhow::Result<()> { + const INSERT_BOOSTED_HEX: &str = r#" + INSERT INTO boosted_hexes ( + "boosts_by_period", "bump_seed", "location", "refreshed_at", "reserved", "version", + + -- our values + "address", + "boost_config", + "created_at", + "device_type", + "start_ts" + ) + VALUES ( + 'ZGRkZGRk', 1, '631798453297853439', '2024-03-12 21:13:53.773+00', '{0,0,0,0,0,0,0,0}', 1, - // Legacy boosted hex with NULL device_type - sqlx::query(INSERT_BOOSTED_HEX) - .bind(Pubkey::new_unique().to_string()) // address - .bind(boost_config_address.to_string()) // boost_config - .bind(now) // created_at - .bind(None as Option) // device_type - .execute(&pool) - .await?; + $1, $2, $3, $4, $5 + ) + "#; - // Boosted hex with new device types - for device_type in &["cbrsIndoor", "cbrsOutdoor", "wifiIndoor", "wifiOutdoor"] { sqlx::query(INSERT_BOOSTED_HEX) .bind(Pubkey::new_unique().to_string()) // address .bind(boost_config_address.to_string()) // boost_config - .bind(now) // created_at - .bind(serde_json::json!(device_type)) // device_type - .execute(&pool) + .bind(created_at) // created_at + .bind(device_type) // device_type + .bind(start_ts.map(|t| t.timestamp()).unwrap_or_default()) // start_ts + .execute(pool) .await?; - } - let count: i64 = sqlx::query_scalar("select count(*) from boosted_hexes") - .fetch_one(&pool) - .await?; - assert_eq!(5, count, "there should be 1 of each type of boosted hex"); - - let mut infos = super::db::all_info_stream(&pool); - let mut print_count = 0; - while let Some(_info) = infos.next().await { - // println!("info: {_info:?}"); - print_count += 1; + Ok(()) } - assert_eq!(5, print_count, "not all rows were able to parse"); - - Ok(()) - } + async fn boosted_hexes_count(pool: &PgPool) -> anyhow::Result { + let count: i64 = sqlx::query_scalar("select count(*) from boosted_hexes") + .fetch_one(pool) + .await?; + Ok(count) + } - fn parse_dt(dt: &str) -> DateTime { - NaiveDateTime::parse_from_str(dt, "%Y-%m-%d %H:%M:%S") - .expect("unable_to_parse") - .and_utc() + async fn streamed_hexes_count(pool: &PgPool) -> anyhow::Result { + // If a row cannot be parsed, it is dropped from the stream with no erros. + let mut infos = super::db::all_info_stream(pool); + let mut count = 0; + while let Some(_info) = infos.next().await { + // println!("info: {_info:?}"); + count += 1; + } + Ok(count) + } } } From 8767a159d42e90b0be72bd38acb8096276d5aa95 Mon Sep 17 00:00:00 2001 From: Michael Jeffrey Date: Wed, 1 May 2024 11:58:24 -0700 Subject: [PATCH 4/8] remove expired boosted hexes when streaming from db --- mobile_config/src/boosted_hex_info.rs | 48 ++++++++++++++++++++++- mobile_config/src/hex_boosting_service.rs | 2 +- 2 files changed, 47 insertions(+), 3 deletions(-) diff --git a/mobile_config/src/boosted_hex_info.rs b/mobile_config/src/boosted_hex_info.rs index 751f05b81..addd56ca2 100644 --- a/mobile_config/src/boosted_hex_info.rs +++ b/mobile_config/src/boosted_hex_info.rs @@ -110,6 +110,10 @@ impl BoostedHexInfo { fn matches_device_type(&self, device_type: &BoostedHexDeviceType) -> bool { self.device_type == *device_type || self.device_type == BoostedHexDeviceType::All } + + fn is_expired(&self, ts: &DateTime) -> bool { + self.end_ts.is_some_and(|end| &end < ts) + } } fn device_type_from_str(s: &str) -> anyhow::Result { @@ -246,6 +250,7 @@ impl BoostedHexes { pub(crate) mod db { use super::{to_end_ts, to_start_ts, BoostedHexInfo}; use chrono::{DateTime, Duration, Utc}; + use futures::future; use futures::stream::{Stream, StreamExt}; use hextree::Cell; use solana_sdk::pubkey::Pubkey; @@ -284,12 +289,20 @@ pub(crate) mod db { where hexes.refreshed_at > $1 "#; - pub fn all_info_stream<'a>( + pub fn all_info_stream_with_time_now<'a>( + db: impl PgExecutor<'a> + 'a, + ) -> impl Stream + 'a { + all_info_stream(db, Utc::now()) + } + + fn all_info_stream<'a>( db: impl PgExecutor<'a> + 'a, + now: DateTime, ) -> impl Stream + 'a { sqlx::query_as::<_, BoostedHexInfo>(GET_BOOSTED_HEX_INFO_SQL) .fetch(db) .filter_map(|info| async move { info.ok() }) + .filter(move |info| future::ready(!info.is_expired(&now))) .boxed() } @@ -593,6 +606,37 @@ mod tests { Ok(()) } + #[sqlx::test] + async fn filter_expired_boosted_hexes(pool: PgPool) -> anyhow::Result<()> { + let boost_config_address = Pubkey::new_unique(); + let now = Utc::now(); + + create_tables(&pool).await?; + insert_boost_config(&pool, &boost_config_address, now).await?; + + let times = vec![ + None, // unstarted + Some(now), // still boosting + Some(now - Duration::days(400)), // expired + ]; + + for time in times { + insert_boosted_hex( + &pool, + &boost_config_address, + now, + Some(serde_json::json!("cbrsIndoor")), + time, + ) + .await?; + } + + assert_eq!(3, boosted_hexes_count(&pool).await?); + assert_eq!(2, streamed_hexes_count(&pool).await?); + + Ok(()) + } + async fn create_tables(pool: &PgPool) -> anyhow::Result<()> { const CREATE_BOOSTED_HEXES_TABLE: &str = r#" CREATE TABLE @@ -724,7 +768,7 @@ mod tests { async fn streamed_hexes_count(pool: &PgPool) -> anyhow::Result { // If a row cannot be parsed, it is dropped from the stream with no erros. - let mut infos = super::db::all_info_stream(pool); + let mut infos = super::db::all_info_stream_with_time_now(pool); let mut count = 0; while let Some(_info) = infos.next().await { // println!("info: {_info:?}"); diff --git a/mobile_config/src/hex_boosting_service.rs b/mobile_config/src/hex_boosting_service.rs index 77a56ed23..167e674ce 100644 --- a/mobile_config/src/hex_boosting_service.rs +++ b/mobile_config/src/hex_boosting_service.rs @@ -70,7 +70,7 @@ impl mobile_config::HexBoosting for HexBoostingService { let (tx, rx) = tokio::sync::mpsc::channel(100); tokio::spawn(async move { - let stream = boosted_hex_info::db::all_info_stream(&pool); + let stream = boosted_hex_info::db::all_info_stream_with_time_now(&pool); stream_multi_info(stream, tx.clone(), signing_key.clone(), batch_size).await }); From fb80d444b2c8170e303c4775abdf0371b4ca5190 Mon Sep 17 00:00:00 2001 From: Michael Jeffrey Date: Wed, 1 May 2024 12:25:00 -0700 Subject: [PATCH 5/8] ensure no tests are written with expired boosted hexes --- mobile_config/src/boosted_hex_info.rs | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/mobile_config/src/boosted_hex_info.rs b/mobile_config/src/boosted_hex_info.rs index addd56ca2..fb7224d24 100644 --- a/mobile_config/src/boosted_hex_info.rs +++ b/mobile_config/src/boosted_hex_info.rs @@ -227,6 +227,14 @@ impl BoostedHexes { } pub fn insert(&mut self, info: BoostedHexInfo) { + #[cfg(test)] + if info.is_expired(&Utc::now()) { + // mobile-config does not deliver expired boosts from the database. + // Tests using this struct to mimic mobile-config should uphold the + // same contract. + panic!("BoostedHexes should not contain expired boosts"); + } + self.hexes.entry(info.location).or_default().push(info); } @@ -418,23 +426,11 @@ mod tests { version: 0, device_type: BoostedHexDeviceType::CbrsIndoor, }, - // Expired boosts should not be considered - BoostedHexInfo { - location: cell, - start_ts: Some(now - Duration::days(60)), - end_ts: Some(now - Duration::days(30)), - period_length: Duration::seconds(2592000), - multipliers: vec![NonZeroU32::new(999).unwrap()], - boosted_hex_pubkey: Pubkey::from_str(BOOST_HEX_PUBKEY)?, - boost_config_pubkey: Pubkey::from_str(BOOST_HEX_CONFIG_PUBKEY)?, - version: 0, - device_type: BoostedHexDeviceType::All, - }, ]; let boosted_hexes = BoostedHexes::new(hexes); let boosts = boosted_hexes.get(&cell).expect("boosts for test cell"); - assert_eq!(boosts.len(), 3, "a hex can be boosted multiple times"); + assert_eq!(boosts.len(), 2, "a hex can be boosted multiple times"); assert_eq!( boosted_hexes.get_current_multiplier(cell, BoostedHexDeviceType::CbrsIndoor, now), From dde070755046a946e3b0762b16e4943b67ca12c5 Mon Sep 17 00:00:00 2001 From: Michael Jeffrey Date: Wed, 1 May 2024 14:35:44 -0700 Subject: [PATCH 6/8] optimize by computing end_ts in db query Thanks for the query help Brian! By precomputing the end timestamp of a boosted hex, we can not have to stream all the hexes out of the db just to throw them away. --- mobile_config/src/boosted_hex_info.rs | 74 +++++++++++++++++---------- 1 file changed, 47 insertions(+), 27 deletions(-) diff --git a/mobile_config/src/boosted_hex_info.rs b/mobile_config/src/boosted_hex_info.rs index fb7224d24..d269f43fd 100644 --- a/mobile_config/src/boosted_hex_info.rs +++ b/mobile_config/src/boosted_hex_info.rs @@ -40,7 +40,7 @@ impl TryFrom for BoostedHexInfo { .map(NonZeroU32::new) .collect::>>() .ok_or_else(|| anyhow::anyhow!("multipliers cannot contain values of 0"))?; - let start_ts = to_start_ts(v.start_ts); + let start_ts = to_timestamp(v.start_ts); let end_ts = to_end_ts(start_ts, period_length, multipliers.len()); let boosted_hex_pubkey: Pubkey = Pubkey::try_from(v.boosted_hex_pubkey.as_slice())?; let boost_config_pubkey: Pubkey = Pubkey::try_from(v.boost_config_pubkey.as_slice())?; @@ -110,10 +110,6 @@ impl BoostedHexInfo { fn matches_device_type(&self, device_type: &BoostedHexDeviceType) -> bool { self.device_type == *device_type || self.device_type == BoostedHexDeviceType::All } - - fn is_expired(&self, ts: &DateTime) -> bool { - self.end_ts.is_some_and(|end| &end < ts) - } } fn device_type_from_str(s: &str) -> anyhow::Result { @@ -228,7 +224,7 @@ impl BoostedHexes { pub fn insert(&mut self, info: BoostedHexInfo) { #[cfg(test)] - if info.is_expired(&Utc::now()) { + if info.end_ts.is_some_and(|end| end < Utc::now()) { // mobile-config does not deliver expired boosts from the database. // Tests using this struct to mimic mobile-config should uphold the // same contract. @@ -256,9 +252,9 @@ impl BoostedHexes { } pub(crate) mod db { - use super::{to_end_ts, to_start_ts, BoostedHexInfo}; + + use super::{to_timestamp, BoostedHexInfo}; use chrono::{DateTime, Duration, Utc}; - use futures::future; use futures::stream::{Stream, StreamExt}; use hextree::Cell; use solana_sdk::pubkey::Pubkey; @@ -268,33 +264,57 @@ pub(crate) mod db { use std::str::FromStr; const GET_BOOSTED_HEX_INFO_SQL: &str = r#" - select - CAST(hexes.location as bigint), - CAST(hexes.start_ts as bigint), + WITH boosted_hexes_replacement AS ( + SELECT + h.*, + CASE + WHEN start_ts = 0 THEN 0 + ELSE h.start_ts + (c.period_length * length(h.boosts_by_period)) + END AS end_ts + FROM boost_configs c + INNER JOIN boosted_hexes h ON c.address = h.boost_config + ) + SELECT + CAST(hexes.location as bigint), + CAST(hexes.start_ts as bigint), + CAST(hexes.end_ts as bigint), config.period_length, hexes.boosts_by_period as multipliers, - hexes.address as boosted_hex_pubkey, + hexes.address as boosted_hex_pubkey, config.address as boost_config_pubkey, hexes.version, hexes.device_type - from boosted_hexes hexes + from boosted_hexes_replacement hexes join boost_configs config on hexes.boost_config = config.address + WHERE + hexes.start_ts = 0 + OR hexes.end_ts > date_part('epoch', $1) "#; - // TODO: reuse with string above + // NOTE(mj): modified hexes should be returned regardless of expiration status const GET_MODIFIED_BOOSTED_HEX_INFO_SQL: &str = r#" - select - CAST(hexes.location as bigint), - CAST(hexes.start_ts as bigint), + WITH boosted_hexes_replacement AS ( + SELECT + h.*, + CASE + WHEN start_ts = 0 THEN 0 + ELSE h.start_ts + (c.period_length * length(h.boosts_by_period)) + END AS end_ts + FROM boost_configs c + INNER JOIN boosted_hexes h ON c.address = h.boost_config + ) + SELECT + CAST(hexes.location AS bigint), + CAST(hexes.start_ts AS bigint), config.period_length, - hexes.boosts_by_period as multipliers, - hexes.address as boosted_hex_pubkey, - config.address as boost_config_pubkey, + hexes.boosts_by_period AS multipliers, + hexes.address AS boosted_hex_pubkey, + config.address AS boost_config_pubkey, hexes.version, hexes.device_type - from boosted_hexes hexes - join boost_configs config on hexes.boost_config = config.address - where hexes.refreshed_at > $1 + FROM boosted_hexes_replacement hexes + JOIN boost_configs config ON hexes.boost_config = config.address + WHERE hexes.refreshed_at > $1 "#; pub fn all_info_stream_with_time_now<'a>( @@ -308,9 +328,9 @@ pub(crate) mod db { now: DateTime, ) -> impl Stream + 'a { sqlx::query_as::<_, BoostedHexInfo>(GET_BOOSTED_HEX_INFO_SQL) + .bind(now) .fetch(db) .filter_map(|info| async move { info.ok() }) - .filter(move |info| future::ready(!info.is_expired(&now))) .boxed() } @@ -328,7 +348,7 @@ pub(crate) mod db { impl sqlx::FromRow<'_, sqlx::postgres::PgRow> for BoostedHexInfo { fn from_row(row: &sqlx::postgres::PgRow) -> sqlx::Result { let period_length = Duration::seconds(row.get::("period_length") as i64); - let start_ts = to_start_ts(row.get::("start_ts") as u64); + let start_ts = to_timestamp(row.get::("start_ts") as u64); let multipliers = row .get::, &str>("multipliers") .into_iter() @@ -337,7 +357,7 @@ pub(crate) mod db { .ok_or_else(|| { sqlx::Error::Decode(Box::from("multipliers cannot contain values of 0")) })?; - let end_ts = to_end_ts(start_ts, period_length, multipliers.len()); + let end_ts = to_timestamp(row.get::("end_ts") as u64); let boost_config_pubkey = Pubkey::from_str(row.get::<&str, &str>("boost_config_pubkey")) .map_err(|e| sqlx::Error::Decode(Box::new(e)))?; @@ -371,7 +391,7 @@ pub(crate) mod db { } } -fn to_start_ts(timestamp: u64) -> Option> { +fn to_timestamp(timestamp: u64) -> Option> { if timestamp == 0 { None } else { From 18fc99a552effeba3607b137f221dc703cd61ef1 Mon Sep 17 00:00:00 2001 From: Michael Jeffrey Date: Mon, 6 May 2024 11:29:38 -0700 Subject: [PATCH 7/8] fixup after rebase - remove unused imports - remove old refactor return types --- .../tests/integrations/watcher_tests.rs | 5 +---- mobile_config/src/boosted_hex_info.rs | 16 ---------------- .../tests/integrations/hex_boosting.rs | 5 +---- 3 files changed, 2 insertions(+), 24 deletions(-) diff --git a/boost_manager/tests/integrations/watcher_tests.rs b/boost_manager/tests/integrations/watcher_tests.rs index 78d180eca..a403efab7 100644 --- a/boost_manager/tests/integrations/watcher_tests.rs +++ b/boost_manager/tests/integrations/watcher_tests.rs @@ -2,10 +2,7 @@ use crate::common::{self, MockFileSinkReceiver, MockHexBoostingClient}; use boost_manager::watcher::{self, Watcher}; use chrono::{Duration as ChronoDuration, Duration, Utc}; use helium_proto::BoostedHexInfoV1 as BoostedHexInfoProto; -use mobile_config::{ - boosted_hex_info::{BoostedHexDeviceType, BoostedHexInfo, BoostedHexInfoStream}, - client::{hex_boosting_client::HexBoostingInfoResolver, ClientError}, -}; +use mobile_config::boosted_hex_info::{BoostedHexDeviceType, BoostedHexInfo}; use solana_sdk::pubkey::Pubkey; use sqlx::PgPool; use std::{num::NonZeroU32, str::FromStr}; diff --git a/mobile_config/src/boosted_hex_info.rs b/mobile_config/src/boosted_hex_info.rs index d269f43fd..85f098df3 100644 --- a/mobile_config/src/boosted_hex_info.rs +++ b/mobile_config/src/boosted_hex_info.rs @@ -233,22 +233,6 @@ impl BoostedHexes { self.hexes.entry(info.location).or_default().push(info); } - - pub fn count(&self) -> usize { - self.hexes.len() - } - - pub fn iter_hexes(&self) -> impl Iterator { - self.hexes.values() - } - - pub fn get(&self, location: &Cell) -> Option<&BoostedHexInfo> { - self.hexes.get(location) - } - - pub fn insert(&mut self, info: BoostedHexInfo) { - self.hexes.insert(info.location, info); - } } pub(crate) mod db { diff --git a/mobile_verifier/tests/integrations/hex_boosting.rs b/mobile_verifier/tests/integrations/hex_boosting.rs index 856a14bd9..3ec1d584e 100644 --- a/mobile_verifier/tests/integrations/hex_boosting.rs +++ b/mobile_verifier/tests/integrations/hex_boosting.rs @@ -11,10 +11,7 @@ use helium_proto::services::poc_mobile::{ UnallocatedReward, }; use hextree::Cell; -use mobile_config::{ - boosted_hex_info::{BoostedHexDeviceType, BoostedHexInfo, BoostedHexInfoStream}, - client::{hex_boosting_client::HexBoostingInfoResolver, ClientError}, -}; +use mobile_config::boosted_hex_info::{BoostedHexDeviceType, BoostedHexInfo}; use mobile_verifier::{ cell_type::CellType, coverage::{set_oracle_boosting_assignments, CoverageObject, UnassignedHex}, From 66b36d80463f9d339bae97e8a4381c5fd118fee5 Mon Sep 17 00:00:00 2001 From: Michael Jeffrey Date: Tue, 7 May 2024 13:21:42 -0700 Subject: [PATCH 8/8] make boosted hex test function more explicit If the expired check had been made a global check, the ability to use BoostedHexes for modified hexes would have broken at runtime. The attempt here is to make very explicit during testing how to meet the same contract as the database queries for boosted hexes. I think there are still some cracks, but we can narrow in on those as we find them. For now, I think naming test constructor functions is a good start. --- boost_manager/src/activator.rs | 2 +- .../tests/integrations/activator_tests.rs | 6 +- mobile_config/src/boosted_hex_info.rs | 24 +++---- mobile_verifier/src/rewarder.rs | 2 +- .../tests/integrations/modeled_coverage.rs | 71 ++++++++++--------- 5 files changed, 52 insertions(+), 53 deletions(-) diff --git a/boost_manager/src/activator.rs b/boost_manager/src/activator.rs index 1dbc9e981..d51586e80 100644 --- a/boost_manager/src/activator.rs +++ b/boost_manager/src/activator.rs @@ -96,7 +96,7 @@ where manifest: RewardManifest, ) -> Result<()> { // get latest boosted hexes info from mobile config - let boosted_hexes = BoostedHexes::get_all(&self.hex_boosting_client).await?; + let boosted_hexes = BoostedHexes::get_active(&self.hex_boosting_client).await?; // get the rewards file from the manifest let manifest_time = manifest.end_timestamp; diff --git a/boost_manager/tests/integrations/activator_tests.rs b/boost_manager/tests/integrations/activator_tests.rs index 0212fcc26..b59ff4e4d 100644 --- a/boost_manager/tests/integrations/activator_tests.rs +++ b/boost_manager/tests/integrations/activator_tests.rs @@ -87,7 +87,7 @@ impl TestContext { async fn test_activated_hex_insert(pool: PgPool) -> anyhow::Result<()> { let now = Utc::now(); let ctx = TestContext::setup(now)?; - let boosted_hexes = BoostedHexes::new(ctx.boosted_hexes); + let boosted_hexes = BoostedHexes::test_new_active(ctx.boosted_hexes)?; // test a boosted hex derived from radio rewards // with a non set start date, will result in a row being @@ -117,7 +117,7 @@ async fn test_activated_hex_insert(pool: PgPool) -> anyhow::Result<()> { async fn test_activated_hex_no_insert(pool: PgPool) -> anyhow::Result<()> { let now = Utc::now(); let ctx = TestContext::setup(now)?; - let boosted_hexes = BoostedHexes::new(ctx.boosted_hexes); + let boosted_hexes = BoostedHexes::test_new_active(ctx.boosted_hexes)?; // test a boosted hex derived from radio rewards // with an active start date, will result in no row being @@ -143,7 +143,7 @@ async fn test_activated_hex_no_insert(pool: PgPool) -> anyhow::Result<()> { async fn test_activated_dup_hex_insert(pool: PgPool) -> anyhow::Result<()> { let now = Utc::now().with_second(0).unwrap(); let ctx = TestContext::setup(now)?; - let boosted_hexes = BoostedHexes::new(ctx.boosted_hexes); + let boosted_hexes = BoostedHexes::test_new_active(ctx.boosted_hexes)?; // test with DUPLICATE boosted hexes derived from radio rewards // with a non set start date, will result in a single row being diff --git a/mobile_config/src/boosted_hex_info.rs b/mobile_config/src/boosted_hex_info.rs index 85f098df3..6bf08421d 100644 --- a/mobile_config/src/boosted_hex_info.rs +++ b/mobile_config/src/boosted_hex_info.rs @@ -149,15 +149,21 @@ pub struct BoostedHexes { } impl BoostedHexes { - pub fn new(hexes: Vec) -> Self { + pub fn test_new_active(hexes: Vec) -> anyhow::Result { let mut me = Self::default(); for hex in hexes { + if hex.end_ts.is_some_and(|end| end < Utc::now()) { + // mobile-config does not deliver expired boosts from the database. + // Tests using this struct to mimic mobile-config should uphold the + // same contract. + panic!("Active BoostedHexes should not contain expired boosts"); + } me.insert(hex); } - me + Ok(me) } - pub async fn get_all( + pub async fn get_active( hex_service_client: &impl HexBoostingInfoResolver, ) -> anyhow::Result { let mut stream = hex_service_client @@ -222,15 +228,7 @@ impl BoostedHexes { self.hexes.get(location) } - pub fn insert(&mut self, info: BoostedHexInfo) { - #[cfg(test)] - if info.end_ts.is_some_and(|end| end < Utc::now()) { - // mobile-config does not deliver expired boosts from the database. - // Tests using this struct to mimic mobile-config should uphold the - // same contract. - panic!("BoostedHexes should not contain expired boosts"); - } - + fn insert(&mut self, info: BoostedHexInfo) { self.hexes.entry(info.location).or_default().push(info); } } @@ -432,7 +430,7 @@ mod tests { }, ]; - let boosted_hexes = BoostedHexes::new(hexes); + let boosted_hexes = BoostedHexes::test_new_active(hexes)?; let boosts = boosted_hexes.get(&cell).expect("boosts for test cell"); assert_eq!(boosts.len(), 2, "a hex can be boosted multiple times"); diff --git a/mobile_verifier/src/rewarder.rs b/mobile_verifier/src/rewarder.rs index 70989003a..7eaa0a6e4 100644 --- a/mobile_verifier/src/rewarder.rs +++ b/mobile_verifier/src/rewarder.rs @@ -391,7 +391,7 @@ async fn reward_poc( speedtest_averages.write_all(speedtest_avg_sink).await?; - let boosted_hexes = BoostedHexes::get_all(hex_service_client).await?; + let boosted_hexes = BoostedHexes::get_active(hex_service_client).await?; let verified_radio_thresholds = radio_threshold::verified_radio_thresholds(pool, reward_period).await?; diff --git a/mobile_verifier/tests/integrations/modeled_coverage.rs b/mobile_verifier/tests/integrations/modeled_coverage.rs index 5f62eadf9..a4cec3979 100644 --- a/mobile_verifier/tests/integrations/modeled_coverage.rs +++ b/mobile_verifier/tests/integrations/modeled_coverage.rs @@ -844,41 +844,42 @@ async fn scenario_three(pool: PgPool) -> anyhow::Result<()> { averages.insert(owner_6.clone(), SpeedtestAverage::from(speedtests_6)); let speedtest_avgs = SpeedtestAverages { averages }; - let mut boosted_hexes = BoostedHexes::default(); - boosted_hexes.insert(BoostedHexInfo { - location: Cell::from_raw(0x8a1fb466d2dffff)?, - start_ts: None, - end_ts: None, - period_length: Duration::hours(1), - multipliers: vec![NonZeroU32::new(1).unwrap()], - boosted_hex_pubkey: Pubkey::from_str(BOOST_HEX_PUBKEY).unwrap(), - boost_config_pubkey: Pubkey::from_str(BOOST_CONFIG_PUBKEY).unwrap(), - version: 0, - device_type: BoostedHexDeviceType::All, - }); - boosted_hexes.insert(BoostedHexInfo { - location: Cell::from_raw(0x8a1fb49642dffff)?, - start_ts: None, - end_ts: None, - period_length: Duration::hours(1), - multipliers: vec![NonZeroU32::new(2).unwrap()], - boosted_hex_pubkey: Pubkey::from_str(BOOST_HEX_PUBKEY).unwrap(), - boost_config_pubkey: Pubkey::from_str(BOOST_CONFIG_PUBKEY).unwrap(), - version: 0, - device_type: BoostedHexDeviceType::All, - }); - boosted_hexes.insert(BoostedHexInfo { - // hotspot 1's location - location: Cell::from_raw(0x8c2681a306607ff)?, - start_ts: None, - end_ts: None, - period_length: Duration::hours(1), - multipliers: vec![NonZeroU32::new(3).unwrap()], - boosted_hex_pubkey: Pubkey::from_str(BOOST_HEX_PUBKEY).unwrap(), - boost_config_pubkey: Pubkey::from_str(BOOST_CONFIG_PUBKEY).unwrap(), - version: 0, - device_type: BoostedHexDeviceType::All, - }); + let boosted_hexes = BoostedHexes::test_new_active(vec![ + BoostedHexInfo { + location: Cell::from_raw(0x8a1fb466d2dffff)?, + start_ts: None, + end_ts: None, + period_length: Duration::hours(1), + multipliers: vec![NonZeroU32::new(1).unwrap()], + boosted_hex_pubkey: Pubkey::from_str(BOOST_HEX_PUBKEY).unwrap(), + boost_config_pubkey: Pubkey::from_str(BOOST_CONFIG_PUBKEY).unwrap(), + version: 0, + device_type: BoostedHexDeviceType::All, + }, + BoostedHexInfo { + location: Cell::from_raw(0x8a1fb49642dffff)?, + start_ts: None, + end_ts: None, + period_length: Duration::hours(1), + multipliers: vec![NonZeroU32::new(2).unwrap()], + boosted_hex_pubkey: Pubkey::from_str(BOOST_HEX_PUBKEY).unwrap(), + boost_config_pubkey: Pubkey::from_str(BOOST_CONFIG_PUBKEY).unwrap(), + version: 0, + device_type: BoostedHexDeviceType::All, + }, + BoostedHexInfo { + // hotspot 1's location + location: Cell::from_raw(0x8c2681a306607ff)?, + start_ts: None, + end_ts: None, + period_length: Duration::hours(1), + multipliers: vec![NonZeroU32::new(3).unwrap()], + boosted_hex_pubkey: Pubkey::from_str(BOOST_HEX_PUBKEY).unwrap(), + boost_config_pubkey: Pubkey::from_str(BOOST_CONFIG_PUBKEY).unwrap(), + version: 0, + device_type: BoostedHexDeviceType::All, + }, + ])?; let reward_period = start..end; let heartbeats = HeartbeatReward::validated(&pool, &reward_period);