Rust improvement #9

m30m · 2024-01-12T14:01:32Z

No description provided.

ali-bahjati · 2024-01-12T17:02:16Z

auction-server/src/liquidation_adapter.rs

+        .collect()
+}
+
+pub fn verify_signature(params: liquidation_adapter::LiquidationCallParams) -> Result<(), String> {


Err Result of String is a bit weird. Why not anyhow?

ali-bahjati · 2024-01-12T17:02:29Z

auction-server/src/liquidation_adapter.rs

+    let nonce = params.valid_until;
+    let digest = H256(keccak256(
+        abi::encode_packed(&[data.into_token(), nonce.into_token()]).map_err(|x| x.to_string())?,
+    )); // TODO: Fix unwrap


there is no unwrap left here

ali-bahjati · 2024-01-12T17:04:52Z

auction-server/src/liquidation_adapter.rs

+        .map(|token| liquidation_adapter::TokenQty {
+            token:  token.0,
+            amount: token.1,
+        })


impl From<(Address, U256)> For TokenQty would be nice here and then you can remove the whole function.

anihamde · 2024-01-12T18:53:58Z

beacon/utils/types_liquidation_adapter.py

@@ -6,6 +6,11 @@
 LIQUIDATION_ADAPTER_ADDRESS = "0x2279B7A0a67DB372996a5FaB50D91eAA73d2eBe6"


+class TokenAmount(TypedDict):


let's rename this TokenQty, for consistency with the naming in the solidity contracts? can also use a better name for both, but let's keep the naming consistent

anihamde · 2024-01-12T18:55:25Z

beacon/utils/endpoints.py

-BEACON_SERVER_ENDPOINT_SURFACE = f"{BEACON_SERVER_ENDPOINT}/surface"
-BEACON_SERVER_ENDPOINT_GETOPPS = f"{BEACON_SERVER_ENDPOINT}/getOpps"
+BEACON_SERVER_ENDPOINT_SURFACE = f"{
+    BEACON_SERVER_ENDPOINT}/orders/submit_order"


this is a confusing name, bc submit order almost sounds like something the end user would do. perhaps something like expose/surface opportunities, in coordination with "beacon"?

anihamde · 2024-01-12T18:56:22Z

beacon/utils/endpoints.py

+BEACON_SERVER_ENDPOINT_SURFACE = f"{
+    BEACON_SERVER_ENDPOINT}/orders/submit_order"
+BEACON_SERVER_ENDPOINT_GETOPPS = f"{
+    BEACON_SERVER_ENDPOINT}/orders/fetch_orders"


let's refer to these as opportunities, as opposed to orders. we aren't handling users' orders

anihamde · 2024-01-12T19:03:20Z

auction-server/src/state.rs

@@ -84,16 +31,30 @@ pub struct SimulatedBid {
    // simulation_time:
 }

+#[derive(Clone)]
+pub struct VerifiedOrder {


note on the naming again--we should change this to something like opportunity

anihamde · 2024-01-12T19:18:29Z

auction-server/src/api/marketplace.rs

+};
+
+#[derive(Serialize, Deserialize, ToSchema, Clone)]
+pub struct TokenAmount {


name consistency

anihamde · 2024-01-12T19:37:53Z

auction-server/src/api/marketplace.rs

+                    amount:   amount.to_string(),
+                })
+                .collect(),
+            receipt_tokens: order


can we just make the receipt_tokens and repay_tokens have the same type across Order and VerifiedOrder?

anihamde · 2024-01-12T19:46:30Z

auction-server/src/api/marketplace.rs

+
+    //TODO: Verify if the call actually works
+
+    store.liquidation_store.orders.write().await.insert(


we need to consider the following when deciding on how to store opportunities

users may want to fetch opps by filters, such as repay_tokens, receipt_tokens, contract

how do we want to remove opps once they are no longer eligible? either bc someone performed the liquidation already or bc the vault is no longer stale?

for 1, we may want to store opportunity UUIDs in hash maps keyed by each of those filters. tradeoff is we would have to handle deletion in duplicate cases.

for 2, we may want to have a slow path that is regularly going through the list of UUIDs and removing ones that are stale. perhaps when an opportunity is fulfilled on-chain via liquidation adapter, we also have an event emitted with UUID, so we can track it down and remove it from the store

anihamde · 2024-01-12T19:51:00Z

auction-server/src/api/marketplace.rs

+),)]
+pub async fn submit_order(
+    State(store): State<Arc<Store>>,
+    Json(order): Json<Order>,


this will still have prices in it on the Python side, should remove that field

anihamde · 2024-01-12T20:16:46Z

auction-server/src/api/marketplace.rs

+    //TODO: Verify if the call actually works
+
+    store.liquidation_store.orders.write().await.insert(
+        Uuid::new_v4(),


relatedly, we may want to create a 1:1 mapping from vault liquidation opportunity to ID? this way it becomes easy to update the right UUID if the calldata changes (e.g. update prices). To do this however, we would need an additional field in the Opportunity/Order structs that indicates vault number or something

anihamde · 2024-01-22T18:44:30Z

auction-server/src/state.rs

@@ -32,7 +31,8 @@ pub struct SimulatedBid {
 }

 #[derive(Clone)]
-pub struct VerifiedOrder {
+pub struct VerifiedLiquidationOpportunity {


can we standardize LiquidationOpportunity and VerifiedLiquidationOpportunity to have the same field names (except for id ofc)

anihamde · 2024-01-22T18:48:19Z

auction-server/src/state.rs

@@ -50,7 +50,7 @@ pub struct ChainStore {

 #[derive(Default)]
 pub struct LiquidationStore {
-    pub orders: RwLock<HashMap<OrderId, VerifiedOrder>>,
+    pub opportunities: RwLock<HashMap<PermissionKey, VerifiedLiquidationOpportunity>>,


i think we decided to kick down the road whether to include tokenqty in the dedup, so this looks good to me

anihamde · 2024-01-23T04:06:50Z

auction-server/src/liquidation_adapter.rs

+            .repay_tokens
+            .into_iter()
+            .map(TokenQty::from)
+            .collect(), // TODO: consistent naming across rust, rest, and solidity


i think you can remove this todo

anihamde · 2024-01-23T04:23:43Z

auction-server/src/api/marketplace.rs

-    let bid_amount = U256::from_dec_str(order_bid.bid_amount.as_str())
+    let liquidation = opportunities
+        .get(&opportunity_bid.permission_key)
+        .ok_or(RestError::OpportunityNotFound)?;


i forget if we discussed how we would handle opportunities no longer being liquidatable. how are we going to handle removing them from the server a) once they become stale (but still actionable bc price on-chain is stale), b) once they become stale (and no longer actionable bc price updated on-chain), c) once they are liquidated?

we decided to have a separate loop which validates the liquidations periodically and removes invalid ones. will address separately

anihamde · 2024-01-23T04:32:30Z

auction-server/src/state.rs

@@ -32,7 +31,8 @@ pub struct SimulatedBid {
 }

 #[derive(Clone)]
-pub struct VerifiedOrder {
+pub struct VerifiedLiquidationOpportunity {
+    pub id:             Uuid,


can we standardize the types of the token fields across LiquidationOpportunity and VerifiedLiquidationOpportunity to both be TokenQty vectors?

the main issue here is having U256 in our api structs. When users pass in strings, it is treated as hex strings and not decimal which is not our desired behavior. I didn't find an easy way to fix this so decided to split some structs between X and VerifiedX where the VerifiedX is input-checked and sanitized.

m30m added 4 commits January 12, 2024 15:00

Improved rust endpoints for liquidation marketplace

afe17ee

Update sample config.yaml

fdf3152

Add python requirements.txt

885fa36

Fix pre-commit

c3c6194

ali-bahjati reviewed Jan 12, 2024

View reviewed changes

anihamde reviewed Jan 12, 2024

View reviewed changes

m30m added 6 commits January 22, 2024 15:23

Remove unused imports and improve readability

b26332e

Use the built abi from the solidity folder

9eda885

Rename order to opportunity

14083ac

Remove prices field from python side

dc448b1

Store a single opportunity per permission key

2f9fbc4

Consistent naming

692c056

anihamde approved these changes Jan 23, 2024

View reviewed changes

Consistent naming

d5955cb

m30m merged commit 44cf436 into main Jan 23, 2024
1 check passed

m30m deleted the rust-improv branch January 23, 2024 10:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rust improvement #9

Rust improvement #9

m30m commented Jan 12, 2024

ali-bahjati Jan 12, 2024 •

edited

Loading

ali-bahjati Jan 12, 2024

ali-bahjati Jan 12, 2024 •

edited

Loading

anihamde Jan 12, 2024

anihamde Jan 12, 2024

anihamde Jan 12, 2024

anihamde Jan 12, 2024

anihamde Jan 12, 2024

anihamde Jan 12, 2024

anihamde Jan 12, 2024

anihamde Jan 12, 2024

anihamde Jan 12, 2024

anihamde Jan 22, 2024

anihamde Jan 22, 2024

anihamde Jan 23, 2024

anihamde Jan 23, 2024

m30m Jan 23, 2024

anihamde Jan 23, 2024

m30m Jan 23, 2024

		@@ -6,6 +6,11 @@
		LIQUIDATION_ADAPTER_ADDRESS = "0x2279B7A0a67DB372996a5FaB50D91eAA73d2eBe6"


		class TokenAmount(TypedDict):


		//TODO: Verify if the call actually works

		store.liquidation_store.orders.write().await.insert(

Rust improvement #9

Rust improvement #9

Conversation

m30m commented Jan 12, 2024

ali-bahjati Jan 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ali-bahjati Jan 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ali-bahjati Jan 12, 2024 •

edited

Loading

ali-bahjati Jan 12, 2024 •

edited

Loading