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(trade): Block users from trading with an old app version #2174

Merged
merged 6 commits into from
Mar 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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
Comment on lines +66 to +70
Copy link
Contributor

Choose a reason for hiding this comment

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

❌ This step already exists a few lines above

- id: set-mobile-version
continue-on-error: true
run: |
cargo set-version --package mobile/native ${{ github.event.inputs.version }}
Copy link
Contributor

Choose a reason for hiding this comment

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

❌ At least locally this does not work.
Correct would be

Suggested change
cargo set-version --package mobile/native ${{ github.event.inputs.version }}
cargo set-version --package native ${{ github.event.inputs.version }}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🙈 I tested it locally with a different command. Thanks for the fix 🙂


- 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
holzeis marked this conversation as resolved.
Show resolved Hide resolved
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 = "dummy@user.com".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>,
holzeis marked this conversation as resolved.
Show resolved Hide resolved
) -> 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