Read write post interactions in db #1482
Conversation
|
Would it make sense to merge this PR to a feature branch instead of |
Seems like a reasonable request. Will leave the PR unmerged for now so others can voice an opinion as well. 👌 |
|
OTOH merging into a feature branch can make the code more stale over time and more difficult to merge later. I'd be OK with merging to |
crates/database/src/orders.rs
Outdated
| pub pre_interactions: Vec<(Address, BigDecimal, Vec<u8>)>, | ||
| pub post_interactions: Vec<(Address, BigDecimal, Vec<u8>)>, | ||
| pub ethflow_data: Option<(Option<TransactionHash>, i64)>, |
There was a problem hiding this comment.
I know this isn't on you, but it would be really nice to get rid of these tuples and either name these fields or give strong types to the tuple elements. It's hard to tell at a glance what the BigDecimal and i64 are for. And since this is a public signature being able to deduce as much as possible at a glance is important.
There was a problem hiding this comment.
Unless I misunderstood the comment on the big SQL query this unfortunate type is due to limitations in sqlx. I created a type alias and attached the related comments to it for now.
There was a problem hiding this comment.
I think you should be able to use a struct with derive(sqlx::FromRow) right? (May be wrong though).
There was a problem hiding this comment.
This currently doesn't work and there is an open issue about it.
Personally, I'm not a huge fan of feature branches. I would rather have a feature flag that can be turned on and off at runtime and merge directly into main. In terms of DB migration, I think we can always "undo" DB changes with additional migrations if needed (not to mention that the DB format for this feature seems relatively well specified and unlikely to change - famous last words and all...) |
Fixes #1481
Adds DB migration to allow differentiating pre- and post-interactions.
Adjust some DB functions to work with pre- and post-interactions.
Reads
post_interactionsfrom DB and puts them intoOrderMetadataand propagates it to some other placesNote that we now only have the required helper functions to store interactions in the DB but there is still no code path that actually calls that code for post-interactions.
Test Plan
Adjusted existing tests for slightly changed DB helper functions
Copied test for verifying roundtrip of pre-interactions for post-interactions