Skip to content

Commit

Permalink
Solution id is string (#3088)
Browse files Browse the repository at this point in the history
# Description
This PR changes the type of the `id` column in the `proposed_solutions`
table from `i64` to `numeric(78,0)`. This adjustment prevents potential
overflow issues from `u64` values in the driver API, which could
previously lead to [failed u64 -> i64
conversions](https://github.com/cowprotocol/services/blob/main/crates/autopilot/src/infra/persistence/mod.rs#L154)
and errors when saving proposed solutions.

Alternative approaches considered:

1. Dropping the `id` column: While feasible, this would prevent reusing
the existing `domain::competition::Solution` object in the autopilot and
would require either creating a new domain object or generating IDs
dynamically when reading the solution from database, both of which are
not great ideas.
2. Changing the API type to `u32`: A more invasive change, requiring
modifications to the driver API and adjustments by solvers (e.g.,
Copium, which sometimes reports IDs exceeding `i64::MAX`).

After evaluating these options, I chose the least disruptive solution of
converting `id` to `numeric(78,0)`.

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

- [ ] Change the type of "id" column, from BigInt to numeric(78,0)

## How to test
Existing tests.

<!--
## Related Issues

Fixes #
-->
  • Loading branch information
sunce86 authored Oct 28, 2024
1 parent c1b102a commit 926a7ac
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 8 deletions.
2 changes: 1 addition & 1 deletion crates/autopilot/src/infra/persistence/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ impl Persistence {
.map(|(uid, participant)| {
let solution = Solution {
uid: uid.try_into().context("uid overflow")?,
id: i64::try_from(participant.solution().id()).context("block overflow")?,
id: u256_to_big_decimal(&participant.solution().id().into()),
solver: ByteArray(participant.solution().solver().0 .0),
is_winner: participant.is_winner(),
score: u256_to_big_decimal(&participant.solution().score().get().0),
Expand Down
12 changes: 6 additions & 6 deletions crates/database/src/solver_competition.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ pub struct Solution {
pub uid: i64,
// Id as reported by the solver (solvers are unaware of how other solvers are numbering their
// solutions)
pub id: i64,
pub id: BigDecimal,
pub solver: Address,
pub is_winner: bool,
pub score: BigDecimal,
Expand Down Expand Up @@ -140,7 +140,7 @@ async fn save_solutions(
builder.push_values(solutions.iter(), |mut b, solution| {
b.push_bind(auction_id)
.push_bind(solution.uid)
.push_bind(solution.id)
.push_bind(&solution.id)
.push_bind(solution.solver)
.push_bind(solution.is_winner)
.push_bind(&solution.score)
Expand Down Expand Up @@ -244,7 +244,7 @@ pub async fn fetch(
#[derive(sqlx::FromRow)]
struct Row {
uid: i64,
id: i64,
id: BigDecimal,
solver: Address,
is_winner: bool,
score: BigDecimal,
Expand Down Expand Up @@ -410,21 +410,21 @@ mod tests {
let solutions = vec![
Solution {
uid: 0,
id: 0,
id: 0.into(),
solver: ByteArray([1u8; 20]), // from solver 1
orders: vec![Default::default()],
..Default::default()
},
Solution {
uid: 1,
id: 0,
id: 0.into(),
solver: ByteArray([2u8; 20]), // from solver 2
orders: vec![Default::default()],
..Default::default()
},
Solution {
uid: 2,
id: 1,
id: 1.into(),
solver: ByteArray([2u8; 20]), // from solver 2
orders: vec![
Order {
Expand Down
2 changes: 1 addition & 1 deletion database/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ All solutions reported by solvers, that were part of a solver competition. A sol
---------------|-----------|----------|--------
auction\_id | bigint | not null | auction for which the solution was proposed
uid | bigint | not null | unique id of the proposed solution within a single auction
id | bytea | not null | id of the proposed solution as reported by the solver
id | numeric | not null | id of the proposed solution as reported by the solver
solver | bytea | not null | solver submission address
is\_winner | boolean | not null | specifies if a solver that proposed this solution is required to execute the solution
score | numeric | not null | score of a solution, based on a scoring criteria used at the time of competition
Expand Down
3 changes: 3 additions & 0 deletions database/sql/V074__drop_solution_id.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
-- Change the type of "id" column in proposed_solutions table from bigint to text
ALTER TABLE proposed_solutions
ALTER COLUMN id TYPE numeric(78,0) USING id::numeric(78,0);

0 comments on commit 926a7ac

Please sign in to comment.