feat: broker can encode btc smart contract call

[j4m1ef0rd]

Pull Request

Closes: PRO-1648

Checklist

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

• I am confident that the code works.
• I have written sufficient tests.
• I have written and tested required migrations.
• I have updated documentation where appropriate.

Summary

• new broker_encodeBtcSmartContractCall rpc that will create an encoded btc start contract call with the given params.
• Extended the SwapLimitsProvider trait to validate the dca and refund params.
  • Added an RPC to use this param validation so that the broker api can use it.

TODO

• Documentation

[codecov]

Codecov Report

Attention: Patch coverage is 22.28916% with 129 lines in your changes missing coverage. Please review.

Project coverage is 71%. Comparing base (d59eac6) to head (d8661eb).
Report is 7 commits behind head on main.

Files with missing lines	Patch %	Lines
api/lib/src/lib.rs	0%	53 Missing ⚠️
...ate-chain/traits/src/mocks/swap_limits_provider.rs	0%	26 Missing ⚠️
state-chain/custom-rpc/src/lib.rs	0%	23 Missing ⚠️
...ne/src/state_chain_observer/client/base_rpc_api.rs	0%	12 Missing ⚠️
state-chain/pallets/cf-swapping/src/lib.rs	69%	11 Missing ⚠️
state-chain/runtime/src/lib.rs	50%	0 Missing and 4 partials ⚠️

Additional details and impacted files

@@          Coverage Diff           @@
##            main   #5329    +/-   ##
======================================
- Coverage     71%     71%    -0%     
======================================
  Files        490     492     +2     
  Lines      85209   85614   +405     
  Branches   85209   85614   +405     
======================================
+ Hits       60659   60917   +258     
- Misses     21837   21964   +127     
- Partials    2713    2733    +20     

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

[dandanlen]

Some minor comments, looking good.

[msgmaxim]

Looks good (apart from things that Dan already commented on)

[dandanlen]

Adding the broker fees is a nice idea but we should not just silently ignore them - better to return an error message for now.

[dandanlen]

Adding the broker fees is a nice idea but we should not just silently ignore them - better to return an error message for now.

☝️ Will add this and then merge, thanks @j4m1ef0rd

[dandanlen]

Actually, I'll also rename the params so that they match request_swap_deposit_address as closely as possible - will make documentation much easier.

[CumpsD]

Any reason you didnt keep this (https://github.com/chainflip-io/chainflip-backend/blob/main/api/bin/chainflip-broker-api/src/main.rs#L61-L65) consistent with the order of https://github.com/chainflip-io/chainflip-backend/blob/main/api/bin/chainflip-broker-api/src/main.rs#L41-L44 ?

[CumpsD]

Also, my Rust is rusty, but here you got affiliate_fees.unwrap_or_default(), https://github.com/chainflip-io/chainflip-backend/blob/main/api/bin/chainflip-broker-api/src/main.rs#L119C5-L119C40

While here you dont: https://github.com/chainflip-io/chainflip-backend/blob/main/api/bin/chainflip-broker-api/src/main.rs#L164

[CumpsD]

Out of curiosity, how are refunds handled in this way? Since there is no refund address, where do you get it from?

[j4m1ef0rd]

Out of curiosity, how are refunds handled in this way? Since there is no refund address, where do you get it from?

The refund address is stored in the 3rd output of the bitcoin transaction. Will be explained in the docs.

but here you got affiliate_fees.unwrap_or_default()

We would just unwrao_or_default in the next function. Does not really matter at what point it becomes non optional. You're right, we should be consistent.

Any reason you didnt keep this consistent with the order of

At first it was because I wanted to put the optional params at the end also some of the params where different, so it could not be exactly the same, but Dan changed it. So now there is no reason they are not consistent 😅.

[dandanlen]

Re. consistency:

I tried to keep it as consistent as possible.

There are some necessary differences between requesting a deposit channel vs. requesting a transaction payload.

The specific reason for the change in order in this case is that retry_duration and min_output_amount are non-optional and replace Option<RefundParameters> from the first method. As @j4m1ef0rd mentioned, we keep optional parameters to the end because when passing the parameters in an array, having optional params in the middle of the array creates some weird ambiguities (should it be null? should it be left out? etc.).

Unifying this would have required changing the original api so I opted not to (for now). My main concern was for naming.

In general, however, I would highly recommend using on object rather than an array for passing the parameters, then the order is a non-issue.

[CumpsD]

Object it is! thanks