feat: Affiliate Broker Registration and Lookup for Vault Swaps #5389

msgmaxim · 2024-11-07T06:10:17Z

Pull Request

Closes: PRO-1750, PRO-1751

Summary

Brokers can register affiliates through the swapping pallet to assign them short ids. Brokers are expected to provide short id that they want to use (rather than SC deciding on their behalf), which also means that they can overwrite existing entries if they want (this can be useful since we only support 256 entries). The new AffiliateRegistrationUpdated event includes (among other things) Option<AccountId> for the entry that is being replaced (most likely to be used as a sanity check to see that they haven't overwritten anything).
vault_swap_request extrinsic now takes Option<(Beneficiary<T::AccountId>, Affiliates<AffiliateShortId>)> where the primary broker is AccountId and affiliate brokers are "short ids" (u8). Short affialite ids are converted to full AccountIds using affiliate registry (added a new trait for this).
BTC witnesser now passes decoded broker id and affiliate short ids into the vault_swap_request call. This hasn't been done for other chains, but should be trivial to add ones primary broker id can be decoded there (I believe @albert-llimos is working on this).
A small refactor in the testing/mock code: moved mock_eth and mock_btc under the new mocks module so that they can share code in a more sensible way (they now share the new MockAffiliateRegistry, which I didn't put under cf_traits sincie it is unlikely that it will be used by any other pallet).

codecov · 2024-11-08T03:39:44Z

Codecov Report

Attention: Patch coverage is 81.36095% with 63 lines in your changes missing coverage. Please review.

Project coverage is 71%. Comparing base (420429c) to head (d20abf6).
Report is 3 commits behind head on main.

Files with missing lines	Patch %	Lines
state-chain/pallets/cf-swapping/src/weights.rs	0%	18 Missing ⚠️
engine/src/witness/btc/deposits.rs	0%	17 Missing ⚠️
state-chain/pallets/cf-swapping/src/lib.rs	71%	10 Missing ⚠️
engine/src/witness/arb.rs	0%	6 Missing ⚠️
engine/src/witness/eth.rs	0%	6 Missing ⚠️
state-chain/pallets/cf-ingress-egress/src/lib.rs	91%	3 Missing ⚠️
state-chain/pallets/cf-swapping/src/mock.rs	0%	3 Missing ⚠️

Additional details and impacted files

@@          Coverage Diff           @@
##            main   #5389    +/-   ##
======================================
- Coverage     72%     71%    -0%     
======================================
  Files        487     489     +2     
  Lines      86183   86508   +325     
  Branches   86183   86508   +325     
======================================
+ Hits       61633   61768   +135     
- Misses     21636   21828   +192     
+ Partials    2914    2912     -2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

albert-llimos · 2024-11-08T08:48:35Z

BTC witnesser now passes decoded broker id and affiliate short ids into the vault_swap_request call. This hasn't been done for other chains, but should be trivial to add ones primary broker id can be decoded there (I believe @albert-llimos is working on this).

Yes, I'll get that done.

engine/src/witness/btc/deposits.rs

engine/src/witness/btc/vault_swaps.rs

state-chain/pallets/cf-ingress-egress/src/lib.rs

state-chain/pallets/cf-swapping/src/lib.rs

dandanlen · 2024-11-08T10:15:59Z

state-chain/primitives/src/lib.rs

+pub type AffiliateShortId = u8;
+


We can use define_wrapper_type now to make this a bit more type-safe.

dandanlen · 2024-11-08T10:17:27Z

engine/src/witness/btc/deposits.rs

 				// TODO: provide broker id (along with broker fees) in the call
-				for (_maybe_broker_id, vault_address) in vault_addresses {
+				for (maybe_broker_id, vault_address) in vault_addresses {


The TODO comment can be removed now?

state-chain/pallets/cf-ingress-egress/src/lib.rs

state-chain/pallets/cf-ingress-egress/src/mocks.rs

state-chain/pallets/cf-swapping/src/lib.rs

msgmaxim · 2024-11-12T04:19:33Z

Addressed all comments now @dandanlen. I also added a check that primary broker is registered as a broker. I chose to just ignore broker fields if the broker is unknown (after emitting a new event), which seems like a reasonable fallback behaviour. If we decide to refund instead, I think this can probably done separately.

albert-llimos · 2024-11-12T07:44:51Z

If we decide to refund instead, I think this can probably done separately.

For now I'd use the fallback as there doesn't seem to be any strong argument (or anyone) pushing for refunding instead. The current behavior sounds good to me.

dandanlen

👌

state-chain/pallets/cf-ingress-egress/src/lib.rs

dandanlen · 2024-11-12T11:37:47Z

state-chain/pallets/cf-ingress-egress/src/lib.rs

+							// In case the entry not found, we ignore the entry, but process the
+							// swap (to avoid having to refund it).


dandanlen · 2024-11-12T11:40:06Z

state-chain/pallets/cf-ingress-egress/src/lib.rs

+				Self::deposit_event(Event::<T, I>::UnknownPrimaryBroker {
+					broker_id: primary_broker,
+				});
+				Default::default()


Do we have a Linear issue to deal with this ie. what to do as fallback? We should probably store the data somewhere so we can reject the deposit / trigger a swap.

But we already do trigger a swap. We can consider refunding of course, but I figured this needs some discussion first. Storing which data are you talking about tough?

As discussed I've opened a triage issue PRO-1804

I think we might be able to take the same path as we do for transaction screening.

msgmaxim added 3 commits November 7, 2024 13:39

feat: basic support for affiliate registration

09438ed

feat: affiliate lookup on vault deposit

5ae89b9

feat: combine broker and affiliate into a tuple

fb9ecb8

msgmaxim force-pushed the feat/affiliate-registration branch from fac7b30 to b49bf8b Compare November 8, 2024 03:24

refactor: unify struct for affiliate short id

9d13dce

msgmaxim force-pushed the feat/affiliate-registration branch from b49bf8b to 9d13dce Compare November 8, 2024 03:29

test: test broker affiliate id lookup

a6223e0

msgmaxim marked this pull request as ready for review November 8, 2024 04:55

msgmaxim requested review from kylezs and dandanlen as code owners November 8, 2024 04:55

kylezs reviewed Nov 8, 2024

View reviewed changes

kylezs approved these changes Nov 8, 2024

View reviewed changes

dandanlen reviewed Nov 8, 2024

View reviewed changes

msgmaxim added 4 commits November 11, 2024 11:56

Merge branch 'main' into feat/affiliate-registration

2de1e0e

chore: address minor review comments

703fab2

chore: strong type for AffiliateShortId

f11d05c

feat: UnknownAffiliateBroker event

613a581

albert-llimos mentioned this pull request Nov 11, 2024

Feat: EVM Vault swaps support for Broker and affiliates #5408

Open

4 tasks

msgmaxim force-pushed the feat/affiliate-registration branch from b0d12dd to 613a581 Compare November 12, 2024 01:06

msgmaxim added 4 commits November 12, 2024 12:21

chore: move MockAffiliateRegistry to cf_traits

69df13b

feat: brokers are mandatory for vault swaps

b2d12d3

feat: check broker status in vault swaps

2e50918

Merge branch 'main' into feat/affiliate-registration

0b0d850

refactor: simplify beneficiary collection

36acf8e

dandanlen changed the title ~~Feat: Affiliate Broker Registration and Lookup for Vault Swaps~~ feat: Affiliate Broker Registration and Lookup for Vault Swaps Nov 12, 2024

dandanlen approved these changes Nov 12, 2024

View reviewed changes

chore: rename events

d20abf6

msgmaxim enabled auto-merge November 13, 2024 01:29

msgmaxim added this pull request to the merge queue Nov 13, 2024

Merged via the queue into main with commit 1ee66a6 Nov 13, 2024
49 checks passed

msgmaxim deleted the feat/affiliate-registration branch November 13, 2024 02:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Affiliate Broker Registration and Lookup for Vault Swaps #5389

feat: Affiliate Broker Registration and Lookup for Vault Swaps #5389

msgmaxim commented Nov 7, 2024 •

edited

Loading

codecov bot commented Nov 8, 2024 •

edited

Loading

albert-llimos commented Nov 8, 2024 •

edited

Loading

dandanlen Nov 8, 2024

dandanlen Nov 8, 2024

msgmaxim commented Nov 12, 2024 •

edited

Loading

albert-llimos commented Nov 12, 2024

dandanlen left a comment

dandanlen Nov 12, 2024

dandanlen Nov 12, 2024

msgmaxim Nov 13, 2024 •

edited

Loading

dandanlen Nov 13, 2024

		// In case the entry not found, we ignore the entry, but process the
		// swap (to avoid having to refund it).

feat: Affiliate Broker Registration and Lookup for Vault Swaps #5389

feat: Affiliate Broker Registration and Lookup for Vault Swaps #5389

Conversation

msgmaxim commented Nov 7, 2024 • edited Loading

Pull Request

Summary

codecov bot commented Nov 8, 2024 • edited Loading

Codecov Report

albert-llimos commented Nov 8, 2024 • edited Loading

dandanlen Nov 8, 2024

Choose a reason for hiding this comment

dandanlen Nov 8, 2024

Choose a reason for hiding this comment

msgmaxim commented Nov 12, 2024 • edited Loading

albert-llimos commented Nov 12, 2024

dandanlen left a comment

Choose a reason for hiding this comment

dandanlen Nov 12, 2024

Choose a reason for hiding this comment

dandanlen Nov 12, 2024

Choose a reason for hiding this comment

msgmaxim Nov 13, 2024 • edited Loading

Choose a reason for hiding this comment

dandanlen Nov 13, 2024

Choose a reason for hiding this comment

msgmaxim commented Nov 7, 2024 •

edited

Loading

codecov bot commented Nov 8, 2024 •

edited

Loading

albert-llimos commented Nov 8, 2024 •

edited

Loading

msgmaxim commented Nov 12, 2024 •

edited

Loading

msgmaxim Nov 13, 2024 •

edited

Loading