Sqlx #37
Sqlx #37
Conversation
m30m
commented
Mar 26, 2024
m30m
commented
- sqlx doesn't support nightly, so I switched to the stable toolchain
| creation_time TIMESTAMP NOT NULL, | ||
| permission_key BYTEA NOT NULL, | ||
| chain_id TEXT NOT NULL, | ||
| target_contract BYTEA NOT NULL, |
There was a problem hiding this comment.
may be worth defining our own custom types here and using for the type fields, here Address (i.e. length 20 bytes)
There was a problem hiding this comment.
Ideally, I could add new domains but sqlx wasn't playing nice with these custom types so I added checks per column
| target_contract BYTEA NOT NULL, | ||
| target_calldata BYTEA NOT NULL, | ||
| bid_amount NUMERIC(80, 0) NOT NULL, | ||
| status TEXT NOT NULL, -- pending, lost, submitted |
There was a problem hiding this comment.
make this an enum, https://docs.rs/sqlx/latest/sqlx/trait.Type.html#enumeration
There was a problem hiding this comment.
I made it an enum but it seems that it's not playing so nice on the rust side:
launchbadge/sqlx#1004 (comment)
auction-server/src/state.rs
Outdated
| Submitted(H256), | ||
| /// The bid lost the auction | ||
| Lost, | ||
| } | ||
|
|
||
| impl BidStatus { | ||
| pub fn status_name(&self) -> String { |
There was a problem hiding this comment.
maybe just implement From<String> and that'll give you the Into?
There was a problem hiding this comment.
Yeah, I didn't want to do that since one variant has more info to be displayed but we don't need that in our sql table. It's not ideal if you log the BidStatus and just see the Submitted variant without the actual hash.
| pub event_sender: broadcast::Sender<UpdateEvent>, | ||
| pub struct Store { | ||
| pub chains: HashMap<ChainId, ChainStore>, | ||
| pub bids: RwLock<HashMap<BidId, SimulatedBid>>, |
There was a problem hiding this comment.
may make sense to create a separate BidStore object, since any changes to this object struct in the future could then be localized to that class and its methods
There was a problem hiding this comment.
will do so when it gets more complicated
| /// database url for persistent storage | ||
| #[arg(long = "database-url")] | ||
| #[arg(env = "DATABASE_URL")] | ||
| pub database_url: String, |
There was a problem hiding this comment.
need to update this in tilt file as well
auction-server/README.md
Outdated
| to the `.env.example` file and set `DATABASE_URL` to the URL of your PostgreSQL database. This file | ||
| will be picked up by sqlx-cli and cargo scripts when running the checks. | ||
|
|
||
| Install sqlx-cli by running `cargo install sqlx-cli`. Then, run the following command to apply migrations: |
There was a problem hiding this comment.
mind adding a note on which directory to run this from, just for clarity
auction-server/src/state.rs
Outdated
| update: BidStatusWithId, | ||
| ) -> anyhow::Result<()> { | ||
| if update.bid_status == BidStatus::Pending { | ||
| return Err(anyhow::anyhow!("Cannot finalize a pending bid")); |
There was a problem hiding this comment.
this error msg is a bit awkward, maybe just "Bid is still pending"
auction-server/src/state.rs
Outdated
| @@ -290,22 +311,25 @@ impl Store { | |||
| Ok(()) | |||
| } | |||
|
|
|||
| pub async fn set_bid_status_and_broadcast( | |||
| pub async fn finalize_bid_status_and_broadcast( | |||
There was a problem hiding this comment.
a clearer function name might be broadcast_bid_status_and_remove since that is less ambiguous as to what finalize means. but i'm fine with current name if you prefer it
| sqlx::query!( | ||
| "UPDATE bid SET status = $1 WHERE id = $2", | ||
| update.bid_status.status_name(), | ||
| "UPDATE bid SET status = $1, removal_time = $2 WHERE id = $3 AND removal_time IS NULL", |
There was a problem hiding this comment.
finalization_time or status_time?
There was a problem hiding this comment.
This field will be removed soon, when we add the auction table. Let's keep it as is for now.
