Skip to content

Commit

Permalink
Fix solver competition loading for multiple winners (#3049)
Browse files Browse the repository at this point in the history
# Description
A breaking change of SolverCompetitionAPI, which now potentially returns
more than 1 transaction associated with the competition (will notify all
teams on slack soon).

Resolves one of the points from
#2830 (comment)

# Changes
<!-- List of detailed changes (how the change is accomplished) -->

- [ ] SolverCompetitionAPI now returns `Vec<TransactionHash>`, instead
of `Option<TransactionHash>`

## How to test
Refactored unit test to prove it works and that no regression issues
were introduced.
Existing e2e test for solver competition proves that competition is
properly returned.
  • Loading branch information
sunce86 authored Oct 14, 2024
1 parent 9619dd2 commit d736be3
Show file tree
Hide file tree
Showing 4 changed files with 94 additions and 63 deletions.
6 changes: 3 additions & 3 deletions crates/autopilot/src/database/competition.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ impl super::Postgres {
deserialize_solver_competition(
row.json,
row.id,
row.tx_hash.map(|hash| H256(hash.0)),
row.tx_hashes.iter().map(|hash| H256(hash.0)).collect(),
)
})
.transpose()
Expand All @@ -159,13 +159,13 @@ impl super::Postgres {
fn deserialize_solver_competition(
json: JsonValue,
auction_id: model::auction::AuctionId,
transaction_hash: Option<H256>,
transaction_hashes: Vec<H256>,
) -> anyhow::Result<SolverCompetitionAPI> {
let common: SolverCompetitionDB =
serde_json::from_value(json).context("deserialize SolverCompetitionDB")?;
Ok(SolverCompetitionAPI {
auction_id,
transaction_hash,
transaction_hashes,
common,
})
}
127 changes: 76 additions & 51 deletions crates/database/src/solver_competition.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,19 +20,21 @@ VALUES ($1, $2)
pub struct LoadCompetition {
pub json: JsonValue,
pub id: AuctionId,
pub tx_hash: Option<TransactionHash>,
// Multiple settlements can be associated with a single competition.
pub tx_hashes: Vec<TransactionHash>,
}

pub async fn load_by_id(
ex: &mut PgConnection,
id: AuctionId,
) -> Result<Option<LoadCompetition>, sqlx::Error> {
const QUERY: &str = r#"
SELECT sc.json, sc.id, s.tx_hash
SELECT sc.json, sc.id, COALESCE(ARRAY_AGG(s.tx_hash) FILTER (WHERE s.tx_hash IS NOT NULL), '{}') AS tx_hashes
FROM solver_competitions sc
-- outer joins because the data might not have been indexed yet
LEFT OUTER JOIN settlements s ON sc.id = s.auction_id
WHERE sc.id = $1
GROUP BY sc.id
;"#;
sqlx::query_as(QUERY).bind(id).fetch_optional(ex).await
}
Expand All @@ -41,10 +43,11 @@ pub async fn load_latest_competition(
ex: &mut PgConnection,
) -> Result<Option<LoadCompetition>, sqlx::Error> {
const QUERY: &str = r#"
SELECT sc.json, sc.id, s.tx_hash
SELECT sc.json, sc.id, COALESCE(ARRAY_AGG(s.tx_hash) FILTER (WHERE s.tx_hash IS NOT NULL), '{}') AS tx_hashes
FROM solver_competitions sc
-- outer joins because the data might not have been indexed yet
LEFT OUTER JOIN settlements s ON sc.id = s.auction_id
GROUP BY sc.id
ORDER BY sc.id DESC
LIMIT 1
;"#;
Expand All @@ -56,10 +59,17 @@ pub async fn load_by_tx_hash(
tx_hash: &TransactionHash,
) -> Result<Option<LoadCompetition>, sqlx::Error> {
const QUERY: &str = r#"
SELECT sc.json, sc.id, s.tx_hash
WITH competition AS (
SELECT sc.id
FROM solver_competitions sc
JOIN settlements s ON sc.id = s.auction_id
WHERE s.tx_hash = $1
)
SELECT sc.json, sc.id, COALESCE(ARRAY_AGG(s.tx_hash) FILTER (WHERE s.tx_hash IS NOT NULL), '{}') AS tx_hashes
FROM solver_competitions sc
JOIN settlements s ON sc.id = s.auction_id
WHERE s.tx_hash = $1
WHERE sc.id = (SELECT id FROM competition)
GROUP BY sc.id
;"#;
sqlx::query_as(QUERY).bind(tx_hash).fetch_optional(ex).await
}
Expand All @@ -70,7 +80,7 @@ mod tests {
super::*,
crate::{
byte_array::ByteArray,
events::{Event, EventIndex, Settlement},
events::{EventIndex, Settlement},
},
sqlx::Connection,
};
Expand All @@ -84,61 +94,76 @@ mod tests {

let value = JsonValue::Bool(true);
save(&mut db, 0, &value).await.unwrap();

// load by id works
let value_ = load_by_id(&mut db, 0).await.unwrap().unwrap();
assert_eq!(value, value_.json);
assert!(value_.tx_hash.is_none());
assert!(value_.tx_hashes.is_empty());
// load as latest works
let value_ = load_latest_competition(&mut db).await.unwrap().unwrap();
assert_eq!(value, value_.json);
assert!(value_.tx_hashes.is_empty());
// load by tx doesn't work, as there is no settlement yet
assert!(load_by_tx_hash(&mut db, &ByteArray([0u8; 32]))
.await
.unwrap()
.is_none());

// non-existent auction returns none
assert!(load_by_id(&mut db, 1).await.unwrap().is_none());
}

#[tokio::test]
#[ignore]
async fn postgres_by_hash() {
let mut db = PgConnection::connect("postgresql://").await.unwrap();
let mut db = db.begin().await.unwrap();
crate::clear_DANGER_(&mut db).await.unwrap();

let id: i64 = 5;
let value = JsonValue::Bool(true);
let hash = ByteArray([1u8; 32]);
save(&mut db, id, &value).await.unwrap();

let value_by_id = load_by_id(&mut db, id).await.unwrap().unwrap();
assert_eq!(value, value_by_id.json);
// no hash because hash columns isn't used to find it
assert!(value_by_id.tx_hash.is_none());

// Fails because the tx_hash stored directly in the solver_competitions table is
// no longer used to look the competition up.
assert!(load_by_tx_hash(&mut db, &hash).await.unwrap().is_none());

// Now insert the proper settlement event and account-nonce.

let index = EventIndex::default();
let event = Event::Settlement(Settlement {
solver: Default::default(),
transaction_hash: hash,
});
crate::events::append(&mut db, &[(index, event)])
.await
.unwrap();

crate::settlements::update_settlement_auction(
// insert two settlement events for the same auction id
crate::events::insert_settlement(
&mut db,
&EventIndex {
block_number: 0,
log_index: 0,
},
&Settlement {
solver: Default::default(),
transaction_hash: ByteArray([0u8; 32]),
},
)
.await
.unwrap();
crate::events::insert_settlement(
&mut db,
index.block_number,
index.log_index,
id,
&EventIndex {
block_number: 0,
log_index: 1,
},
&Settlement {
solver: Default::default(),
transaction_hash: ByteArray([1u8; 32]),
},
)
.await
.unwrap();
crate::settlements::update_settlement_auction(&mut db, 0, 0, 0)
.await
.unwrap();
crate::settlements::update_settlement_auction(&mut db, 0, 1, 0)
.await
.unwrap();

// load by id works, and finds two hashes
let value_ = load_by_id(&mut db, 0).await.unwrap().unwrap();
assert!(value_.tx_hashes.len() == 2);

// Now succeeds.
let value_by_hash = load_by_tx_hash(&mut db, &hash).await.unwrap().unwrap();
assert_eq!(value, value_by_hash.json);
assert_eq!(id, value_by_hash.id);
// load as latest works, and finds two hashes
let value_ = load_latest_competition(&mut db).await.unwrap().unwrap();
assert!(value_.tx_hashes.len() == 2);

// By id also sees the hash now.
let value_by_id = load_by_id(&mut db, id).await.unwrap().unwrap();
assert_eq!(hash, value_by_id.tx_hash.unwrap());
// load by tx works, and finds two hashes, no matter which tx hash is used
let value_ = load_by_tx_hash(&mut db, &ByteArray([0u8; 32]))
.await
.unwrap()
.unwrap();
assert!(value_.tx_hashes.len() == 2);
let value_ = load_by_tx_hash(&mut db, &ByteArray([1u8; 32]))
.await
.unwrap()
.unwrap();
assert!(value_.tx_hashes.len() == 2);
}
}
8 changes: 4 additions & 4 deletions crates/model/src/solver_competition.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ pub struct SolverCompetitionDB {
pub struct SolverCompetitionAPI {
#[serde(default)]
pub auction_id: AuctionId,
pub transaction_hash: Option<H256>,
pub transaction_hashes: Vec<H256>,
#[serde(flatten)]
pub common: SolverCompetitionDB,
}
Expand Down Expand Up @@ -126,7 +126,7 @@ mod tests {
"auctionId": 0,
"auctionStartBlock": 13u64,
"competitionSimulationBlock": 15u64,
"transactionHash": "0x1111111111111111111111111111111111111111111111111111111111111111",
"transactionHashes": ["0x1111111111111111111111111111111111111111111111111111111111111111"],
"auction": {
"orders": [
"0x1111111111111111111111111111111111111111111111111111111111111111\
Expand Down Expand Up @@ -173,7 +173,7 @@ mod tests {

let orig = SolverCompetitionAPI {
auction_id: 0,
transaction_hash: Some(H256([0x11; 32])),
transaction_hashes: vec![H256([0x11; 32])],
common: SolverCompetitionDB {
auction_start_block: 13,
competition_simulation_block: 15,
Expand Down Expand Up @@ -360,7 +360,7 @@ mod tests {
}
}
],
"transactionHash": "0x044499c2a830890cb0a8ecf9aec6c5621e8310092a58d369cdef726254d3d108",
"transactionHashes": ["0x044499c2a830890cb0a8ecf9aec6c5621e8310092a58d369cdef726254d3d108"],
"auctionStartBlock": 15173535,
"liquidityCollectedBlock": 15173535,
"competitionSimulationBlock": 15173535
Expand Down
16 changes: 11 additions & 5 deletions crates/orderbook/src/database/solver_competition.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,13 @@ use {
fn deserialize_solver_competition(
json: JsonValue,
auction_id: AuctionId,
transaction_hash: Option<H256>,
transaction_hashes: Vec<H256>,
) -> Result<SolverCompetitionAPI, LoadSolverCompetitionError> {
let common: SolverCompetitionDB =
serde_json::from_value(json).context("deserialize SolverCompetitionDB")?;
Ok(SolverCompetitionAPI {
auction_id,
transaction_hash,
transaction_hashes,
common,
})
}
Expand All @@ -45,14 +45,20 @@ impl SolverCompetitionStoring for Postgres {
deserialize_solver_competition(
row.json,
row.id,
row.tx_hash.map(|hash| H256(hash.0)),
row.tx_hashes.iter().map(|hash| H256(hash.0)).collect(),
)
}),
Identifier::Transaction(hash) => {
database::solver_competition::load_by_tx_hash(&mut ex, &ByteArray(hash.0))
.await
.context("solver_competition::load_by_tx_hash")?
.map(|row| deserialize_solver_competition(row.json, row.id, Some(hash)))
.map(|row| {
deserialize_solver_competition(
row.json,
row.id,
row.tx_hashes.iter().map(|hash| H256(hash.0)).collect(),
)
})
}
}
.ok_or(LoadSolverCompetitionError::NotFound)?
Expand All @@ -74,7 +80,7 @@ impl SolverCompetitionStoring for Postgres {
deserialize_solver_competition(
row.json,
row.id,
row.tx_hash.map(|hash| H256(hash.0)),
row.tx_hashes.iter().map(|hash| H256(hash.0)).collect(),
)
})
.ok_or(LoadSolverCompetitionError::NotFound)?
Expand Down

0 comments on commit d736be3

Please sign in to comment.