Feat: ingress-egress tracking for DOT #4121

msgmaxim · 2023-10-16T05:35:33Z

Pull Request

Checklist

Please conduct a thorough self-review before opening the PR.

I am confident that the code works.
I have updated documentation where appropriate.

Summary

Took me a while, but I managed to extract the proxy_added_witnessing and process_egress from inside the lamdbas to avoid duplicating logic (it is hard to keep track of types of different variables when everything is implicit, and I needed an extra constraint on ExtraHistoricInfo in dot_deposits to make it work). I then used these functions in the new dot module inside ingress-egress tracking. Tested the engine and the tracking for DOT witnessing on localnet for both ingress and egress.

linear · 2023-10-16T05:35:35Z

PRO-868 Ingress Egress Tracker

This will/should be broken down into smaller tickets after we've discussed it.

Problem:

The swapping app currently has to wait for the CFE witnessing and respective confirmation on the SC, both for ingress, when the user is depositing funds, and for egress, when the user is receiving funds. This duration can be quite long, because of the added safety margins to protect against reorgs.

Solution:

While the reorg safety is important for the protocol it's not important for the product. We can use the same code that we use in the CFE witnessing to avoid duplicating the work, but instead redirect the results of that.

Requirements:

The Calls that we would normally send to the SC as an extrinsic, can instead be sent down a WS stream for consumption by product team (it doesn't have to be WS according to product team, but I think WS will be simplest for us to do)
Reusing the same witnessing components as the engine currently does where possible.
Single binary for the trackers of all the chains - this includes the btc deposit tracker, so would unify / use that deposit tracker binary alongside the code for this
Configurable, so it can be used for different networks
- SC WS endpoint
- A single, (no need for backup) WS and HTTP endpoints for each chain

Useful info:

feat: simple pre-witnessing #4056 / PRO-852
- Example of how the egress components would be pulled out, as well as the usage of the ProcessCall, and how it can be used to pass in closures to send different data

Out of scope:

State, there's no need to hold state in the tracker, either in memory, or in a database. We can be optimistic. If the tracker goes down that's ok, the UI can fallback on the SC BroadcastSuccess event which it already uses, it'll just be a bit slower from the user's perspective for now, but for now that's ok.

If you have questions for the product team and what they need, you can reach out to nat

codecov · 2023-10-16T05:46:52Z

Codecov Report

Merging #4121 (156ba46) into main (5c70808) will decrease coverage by 0%.
Report is 2 commits behind head on main.
The diff coverage is 0%.

@@          Coverage Diff          @@
##            main   #4121   +/-   ##
=====================================
- Coverage     72%     72%   -0%     
=====================================
  Files        378     378           
  Lines      60637   60663   +26     
  Branches   60637   60663   +26     
=====================================
- Hits       43492   43490    -2     
- Misses     14881   14905   +24     
- Partials    2264    2268    +4

Files	Coverage Δ
engine/src/witness/dot/dot_deposits.rs	`62% <0%> (-1%)`	⬇️
engine/src/witness/dot.rs	`25% <0%> (-2%)`	⬇️

... and 3 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

kylezs

👍

kylezs · 2023-10-16T11:23:18Z

api/bin/chainflip-ingress-egress-tracker/src/main.rs

 		},
-		eth_key_path: eth_key_path.into(),
-		state_chain_ws_endpoint: sc_ws_endpoint,
+		state_chain_ws_endpoint: env::var("SC_WS_ENDPOINT")


We don't have the BTC settings loaded in here, we should probably change that to be consistent... it's weird that the settings are read from environment on each call anyway for BTC. - will create an issue, is separate to this PR: https://linear.app/chainflip/issue/PRO-916/btc-ingress-egress-tracker-settings-to-be-unified-into

kylezs · 2023-10-16T11:27:12Z

api/bin/chainflip-ingress-egress-tracker/src/witnessing/dot.rs

+		+ 'static,
+	ProcessingFut: Future<Output = ()> + Send + 'static,
+{
+	let dot_client = {


no need for this block {}

kylezs · 2023-10-16T11:30:19Z

api/bin/chainflip-ingress-egress-tracker/src/witnessing/dot.rs

+		.egress_items(scope, state_chain_stream.clone(), state_chain_client.clone())
+		.await
+		.then({
+			let process_call = witness_call.clone();


nit for consistency

Suggested change

let process_call = witness_call.clone();

let witness_call = witness_call.clone();

kylezs · 2023-10-16T11:30:58Z

api/bin/chainflip-ingress-egress-tracker/src/witnessing/dot.rs

+				process_egress(epoch, header, process_call.clone(), dot_client.clone())
+			}
+		})
+		.logging("DOT Witnessing")


for consistency

Suggested change

.logging("DOT Witnessing")

.logging("witnessing")

the chain is already logged in the logging adapter which is enough for polkadot

* origin/main: chore: get gas parameters from statechain event (#4125) Feat: ingress-egress tracking for DOT (#4121) add support for hex encoded amounts on limit order and range order methods in LP API (#4120) feat: bouncer command for submitting runtime upgrades (#4122) Feat: ensure correct process termination in ingress/egress tracker (#4101) feat(custom-rpc): add flip balance to account info (#4119) chore: storage migration delete NextCompatibilityVersion (#4115) chore: delete unneeded function (#4116) # Conflicts: # state-chain/runtime/src/lib.rs

refactor: extract dot witnessing procedures into reusable functions

fd2a1f8

msgmaxim requested review from kylezs and j4m1ef0rd October 16, 2023 05:35

feat: add dot ingress-egress tracking

51e6ef8

msgmaxim force-pushed the feat/dot-ingress-egress-tracking branch from d5c95eb to 51e6ef8 Compare October 16, 2023 05:37

chore: silence clippy type complexity

9924ae2

kylezs approved these changes Oct 16, 2023

View reviewed changes

chore: address minor review comments

156ba46

msgmaxim enabled auto-merge (squash) October 17, 2023 01:21

msgmaxim merged commit 03c4f3c into main Oct 17, 2023
44 checks passed

msgmaxim deleted the feat/dot-ingress-egress-tracking branch October 17, 2023 01:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat: ingress-egress tracking for DOT #4121

Feat: ingress-egress tracking for DOT #4121

msgmaxim commented Oct 16, 2023

linear bot commented Oct 16, 2023

codecov bot commented Oct 16, 2023 •

edited

Loading

kylezs left a comment

kylezs Oct 16, 2023

kylezs Oct 16, 2023

kylezs Oct 16, 2023

kylezs Oct 16, 2023 •

edited

Loading

kylezs Oct 16, 2023

	let process_call = witness_call.clone();
	let witness_call = witness_call.clone();

Feat: ingress-egress tracking for DOT #4121

Feat: ingress-egress tracking for DOT #4121

Conversation

msgmaxim commented Oct 16, 2023

Pull Request

Checklist

Summary

linear bot commented Oct 16, 2023

codecov bot commented Oct 16, 2023 • edited Loading

Codecov Report

kylezs left a comment

Choose a reason for hiding this comment

kylezs Oct 16, 2023

Choose a reason for hiding this comment

kylezs Oct 16, 2023

Choose a reason for hiding this comment

kylezs Oct 16, 2023

Choose a reason for hiding this comment

kylezs Oct 16, 2023 • edited Loading

Choose a reason for hiding this comment

kylezs Oct 16, 2023

Choose a reason for hiding this comment

codecov bot commented Oct 16, 2023 •

edited

Loading

kylezs Oct 16, 2023 •

edited

Loading