Skip to content

Commit

Permalink
Merge pull request #2174 from get10101/chore/block-users-from-trading…
Browse files Browse the repository at this point in the history
…-with-old-versions

feat(trade): Block users from trading with an old app version
  • Loading branch information
holzeis authored Mar 6, 2024
2 parents 78c95bf + bb70acb commit 34751f4
Show file tree
Hide file tree
Showing 30 changed files with 129 additions and 29 deletions.
14 changes: 12 additions & 2 deletions .github/workflows/draft_new_release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -63,12 +63,22 @@ jobs:
run: |
cargo set-version --package webapp ${{ github.event.inputs.version }}
- name: Commit changelog and manifest files
- name: Bump mobile/native Cargo.toml version
uses: stellar/binaries@v13
with:
name: cargo-edit
version: 0.11.6
- id: set-mobile-version
continue-on-error: true
run: |
cargo set-version --package mobile/native ${{ github.event.inputs.version }}
- name: Commit manifest files
id: make-commit
run: |
/home/runner/.dprint/bin/dprint fmt
git add mobile/pubspec.yaml coordinator/Cargo.toml webapp/Cargo.toml
git add mobile/native/Cargo.toml mobile/pubspec.yaml coordinator/Cargo.toml webapp/Cargo.toml Cargo.lock
git commit --message "Prepare release ${{ github.event.inputs.version }}"
echo "::set-output name=commit::$(git rev-parse HEAD)"
Expand Down
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ALTER TABLE users DROP COLUMN IF EXISTS "version";
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ALTER TABLE users ADD COLUMN "version" TEXT;
21 changes: 21 additions & 0 deletions coordinator/src/check_version.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
use crate::db;
use crate::db::user::User;
use anyhow::ensure;
use anyhow::Context;
use anyhow::Result;
use bitcoin::secp256k1::PublicKey;
use diesel::PgConnection;

pub fn check_version(conn: &mut PgConnection, trader_id: &PublicKey) -> Result<()> {
let user: User = db::user::get_user(conn, trader_id)?.context("Couldn't find user")?;

let app_version = user.version.context("No version found")?;

let coordinator_version = env!("CARGO_PKG_VERSION").to_string();
ensure!(
app_version == coordinator_version,
format!("Please upgrade to the latest version: {coordinator_version}")
);

Ok(())
}
18 changes: 16 additions & 2 deletions coordinator/src/db/user.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ pub struct User {
pub fcm_token: String,
pub last_login: OffsetDateTime,
pub nickname: Option<String>,
// TODO(holzeis): Version is only optional for the first upgrade. Afterwards we should make it
// mandatory.
pub version: Option<String>,
}

impl From<RegisterParams> for User {
Expand All @@ -32,6 +35,7 @@ impl From<RegisterParams> for User {
timestamp: OffsetDateTime::now_utc(),
fcm_token: "".to_owned(),
last_login: OffsetDateTime::now_utc(),
version: value.version,
}
}
}
Expand All @@ -53,6 +57,7 @@ pub fn upsert_user(
trader_id: PublicKey,
contact: Option<String>,
nickname: Option<String>,
version: Option<String>,
) -> QueryResult<User> {
// If no name or contact has been provided we default to empty string
let contact = contact.unwrap_or_default();
Expand All @@ -68,13 +73,15 @@ pub fn upsert_user(
timestamp,
fcm_token: "".to_owned(),
last_login: timestamp,
version: version.clone(),
})
.on_conflict(schema::users::pubkey)
.do_update()
.set((
users::contact.eq(&contact),
users::nickname.eq(&nickname),
users::last_login.eq(timestamp),
users::version.eq(version),
))
.get_result(conn)?;
Ok(user)
Expand Down Expand Up @@ -103,7 +110,12 @@ pub fn update_nickname(
Ok(())
}

pub fn login_user(conn: &mut PgConnection, trader_id: PublicKey, token: String) -> Result<()> {
pub fn login_user(
conn: &mut PgConnection,
trader_id: PublicKey,
token: String,
version: Option<String>,
) -> Result<()> {
tracing::debug!(%trader_id, token, "Updating token for client.");
let last_login = OffsetDateTime::now_utc();
let affected_rows = diesel::insert_into(users::table)
Expand All @@ -114,13 +126,15 @@ pub fn login_user(conn: &mut PgConnection, trader_id: PublicKey, token: String)
nickname: None,
timestamp: OffsetDateTime::now_utc(),
fcm_token: token.clone(),
version: version.clone(),
last_login,
})
.on_conflict(schema::users::pubkey)
.do_update()
.set((
users::fcm_token.eq(&token),
users::last_login.eq(last_login),
users::version.eq(version),
))
.execute(conn)?;

Expand All @@ -132,7 +146,7 @@ pub fn login_user(conn: &mut PgConnection, trader_id: PublicKey, token: String)
Ok(())
}

pub fn get_user(conn: &mut PgConnection, trader_id: PublicKey) -> Result<Option<User>> {
pub fn get_user(conn: &mut PgConnection, trader_id: &PublicKey) -> Result<Option<User>> {
let maybe_user = users::table
.filter(users::pubkey.eq(trader_id.to_string()))
.first(conn)
Expand Down
2 changes: 1 addition & 1 deletion coordinator/src/leaderboard.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ pub(crate) fn generate_leader_board(
let leader_board = leader_board
.into_iter()
.map(|entry| {
let nickname = db::user::get_user(conn, entry.trader).unwrap_or_default();
let nickname = db::user::get_user(conn, &entry.trader).unwrap_or_default();
LeaderBoardEntry {
nickname: nickname.and_then(|user| user.nickname).unwrap_or_default(),
..entry
Expand Down
1 change: 1 addition & 0 deletions coordinator/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ mod payout_curve;
pub mod admin;
pub mod backup;
pub mod campaign;
pub mod check_version;
pub mod cli;
pub mod db;
pub mod dlc_handler;
Expand Down
7 changes: 7 additions & 0 deletions coordinator/src/node/rollover.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::check_version::check_version;
use crate::db;
use crate::db::positions;
use crate::dlc_protocol;
Expand Down Expand Up @@ -168,6 +169,12 @@ impl Node {
.expect("task to complete")?;

tracing::debug!(%trader_id, "Checking if the users positions is eligible for rollover");

if check_version(&mut conn, &trader_id).is_err() {
tracing::info!(%trader_id, "User is not on the latest version. Skipping check if users position is eligible for rollover");
return Ok(());
}

if let Some(position) = positions::Position::get_position_by_trader(
&mut conn,
trader_id,
Expand Down
6 changes: 6 additions & 0 deletions coordinator/src/orderbook/async_match.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::check_version::check_version;
use crate::message::NewUserMessage;
use crate::message::OrderbookMessage;
use crate::orderbook::db::matches;
Expand Down Expand Up @@ -90,6 +91,11 @@ async fn process_pending_match(
.await
.expect("task to complete")?;

if check_version(&mut conn, &trader_id).is_err() {
tracing::info!(%trader_id, "User is not on the latest version. Skipping check if user needs to be informed about pending matches.");
return Ok(());
}

if let Some(order) =
orders::get_by_trader_id_and_state(&mut conn, trader_id, OrderState::Matched)?
{
Expand Down
11 changes: 11 additions & 0 deletions coordinator/src/orderbook/routes.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::check_version::check_version;
use crate::orderbook;
use crate::orderbook::trading::NewOrderMessage;
use crate::orderbook::trading::TradingError;
Expand Down Expand Up @@ -74,6 +75,16 @@ pub async fn post_order(

let new_order = new_order_request.value;

// TODO(holzeis): We should add a similar check eventually for limit orders (makers).
if new_order.order_type == OrderType::Market {
let mut conn = state
.pool
.get()
.map_err(|e| AppError::InternalServerError(e.to_string()))?;
check_version(&mut conn, &new_order.trader_id)
.map_err(|e| AppError::BadRequest(e.to_string()))?;
}

let settings = state.settings.read().await;
if new_order.order_type == OrderType::Limit
&& settings.whitelist_enabled
Expand Down
5 changes: 4 additions & 1 deletion coordinator/src/orderbook/tests/registration_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,16 +22,18 @@ async fn registered_user_is_stored_in_db() {
let dummy_email = "[email protected]".to_string();
let nickname = Some("dummy_user".to_string());
let fcm_token = "just_a_token".to_string();
let version = Some("1.9.0".to_string());

let user = user::upsert_user(
&mut conn,
dummy_pubkey,
Some(dummy_email.clone()),
nickname.clone(),
version.clone(),
)
.unwrap();
assert!(user.id.is_some(), "Id should be filled in by diesel");
user::login_user(&mut conn, dummy_pubkey, fcm_token.clone()).unwrap();
user::login_user(&mut conn, dummy_pubkey, fcm_token.clone(), version.clone()).unwrap();

let users = user::all(&mut conn).unwrap();
assert_eq!(users.len(), 1);
Expand All @@ -40,6 +42,7 @@ async fn registered_user_is_stored_in_db() {
assert_eq!(users.first().unwrap().contact, dummy_email);
assert_eq!(users.first().unwrap().nickname, nickname);
assert_eq!(users.first().unwrap().fcm_token, fcm_token);
assert_eq!(users.first().unwrap().version, version);
}

fn dummy_public_key() -> PublicKey {
Expand Down
3 changes: 2 additions & 1 deletion coordinator/src/orderbook/websocket.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ pub async fn websocket_connection(stream: WebSocket, state: Arc<AppState>) {
}
Ok(OrderbookRequest::Authenticate {
fcm_token,
version,
signature,
}) => {
let msg = create_sign_message(AUTH_SIGN_MESSAGE.to_vec());
Expand Down Expand Up @@ -160,7 +161,7 @@ pub async fn websocket_connection(stream: WebSocket, state: Arc<AppState>) {
}

let token = fcm_token.unwrap_or("unavailable".to_string());
if let Err(e) = user::login_user(&mut conn, trader_id, token) {
if let Err(e) = user::login_user(&mut conn, trader_id, token, version) {
tracing::error!(%trader_id, "Failed to update logged in user. Error: {e:#}")
}

Expand Down
11 changes: 10 additions & 1 deletion coordinator/src/routes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use crate::admin::roll_back_dlc_channel;
use crate::admin::sign_message;
use crate::backup::SledBackup;
use crate::campaign::post_push_campaign;
use crate::check_version::check_version;
use crate::collaborative_revert::confirm_collaborative_revert;
use crate::db;
use crate::db::user;
Expand Down Expand Up @@ -263,6 +264,13 @@ pub async fn post_trade(
State(state): State<Arc<AppState>>,
params: Json<TradeAndChannelParams>,
) -> Result<(), AppError> {
let mut conn = state
.pool
.get()
.map_err(|e| AppError::InternalServerError(e.to_string()))?;
check_version(&mut conn, &params.trade_params.pubkey)
.map_err(|e| AppError::BadRequest(e.to_string()))?;

state
.node
.trade(state.auth_users_notifier.clone(), params.0)
Expand Down Expand Up @@ -337,6 +345,7 @@ pub async fn post_register(
register_params.pubkey,
register_params.contact.clone(),
register_params.nickname.clone(),
register_params.version.clone(),
)
.map_err(|e| AppError::InternalServerError(format!("Could not upsert user: {e:#}")))?;

Expand Down Expand Up @@ -388,7 +397,7 @@ pub async fn get_user(
let trader_pubkey = PublicKey::from_str(trader_pubkey.as_str())
.map_err(|_| AppError::BadRequest("Invalid trader id provided".to_string()))?;

let option = user::get_user(&mut conn, trader_pubkey)
let option = user::get_user(&mut conn, &trader_pubkey)
.map_err(|e| AppError::InternalServerError(format!("Could not load users: {e:#}")))?;

match option {
Expand Down
1 change: 1 addition & 0 deletions coordinator/src/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -374,6 +374,7 @@ diesel::table! {
fcm_token -> Text,
last_login -> Timestamptz,
nickname -> Nullable<Text>,
version -> Nullable<Text>,
}
}

Expand Down
1 change: 1 addition & 0 deletions crates/commons/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ pub struct RegisterParams {
pub pubkey: PublicKey,
pub contact: Option<String>,
pub nickname: Option<String>,
pub version: Option<String>,
}

/// Registration details for enrolling into the beta program
Expand Down
1 change: 1 addition & 0 deletions crates/commons/src/message.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ pub struct LspConfig {
pub enum OrderbookRequest {
Authenticate {
fcm_token: Option<String>,
version: Option<String>,
signature: Signature,
},
LimitOrderFilledMatches {
Expand Down
2 changes: 1 addition & 1 deletion crates/orderbook-client/examples/authenticated.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ async fn main() -> Result<Never> {

loop {
let (_, mut stream) =
orderbook_client::subscribe_with_authentication(url.clone(), &authenticate, None)
orderbook_client::subscribe_with_authentication(url.clone(), &authenticate, None, None)
.await?;

loop {
Expand Down
7 changes: 5 additions & 2 deletions crates/orderbook-client/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ pub async fn subscribe(
SplitSink<WebSocketStream, tungstenite::Message>,
impl Stream<Item = Result<String, anyhow::Error>> + Unpin,
)> {
subscribe_impl(None, url, None).await
subscribe_impl(None, url, None, None).await
}

/// Connects to the orderbook WebSocket API with authentication.
Expand All @@ -33,12 +33,13 @@ pub async fn subscribe_with_authentication(
url: String,
authenticate: impl Fn(Message) -> Signature,
fcm_token: Option<String>,
version: Option<String>,
) -> Result<(
SplitSink<WebSocketStream, tungstenite::Message>,
impl Stream<Item = Result<String, anyhow::Error>> + Unpin,
)> {
let signature = create_auth_message_signature(authenticate);
subscribe_impl(Some(signature), url, fcm_token).await
subscribe_impl(Some(signature), url, fcm_token, version).await
}

pub fn create_auth_message_signature(authenticate: impl Fn(Message) -> Signature) -> Signature {
Expand All @@ -50,6 +51,7 @@ async fn subscribe_impl(
signature: Option<Signature>,
url: String,
fcm_token: Option<String>,
version: Option<String>,
) -> Result<(
SplitSink<WebSocketStream, tungstenite::Message>,
impl Stream<Item = Result<String>> + Unpin,
Expand All @@ -67,6 +69,7 @@ async fn subscribe_impl(
.send(tungstenite::Message::try_from(
OrderbookRequest::Authenticate {
fcm_token,
version,
signature,
},
)?)
Expand Down
5 changes: 2 additions & 3 deletions mobile/lib/features/trade/domain/order.dart
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,6 @@ class FailureReason {
static const FailureReason timeout = FailureReason._(
failureType: FailureReasonType.timeout,
details: "The order timed out before finding a match");
static const FailureReason rejected =
FailureReason._(failureType: FailureReasonType.rejected, details: "The order was rejected.");
static const FailureReason unknown = FailureReason._(
failureType: FailureReasonType.unknown, details: "An unknown error occurred.");

Expand All @@ -90,7 +88,8 @@ class FailureReason {
case bridge.FailureReason_TimedOut():
return timeout;
case bridge.FailureReason_OrderRejected():
return rejected;
return FailureReason._(
failureType: FailureReasonType.rejected, details: failureReason.field0);
case bridge.FailureReason_Unknown():
return unknown;
}
Expand Down
2 changes: 1 addition & 1 deletion mobile/native/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "native"
version = "0.1.0"
version = "1.9.0"
edition = "2021"

[lib]
Expand Down
Loading

0 comments on commit 34751f4

Please sign in to comment.