-
Notifications
You must be signed in to change notification settings - Fork 23
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Apply a funding fee on rollover #2532
Conversation
b5cbeeb
to
17013e7
Compare
6003175
to
1b319fd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pretty awesome. Looking forward to having this merged. Added a few comments and found one potential bug.
#2532 (comment)
coordinator/migrations/2024-05-01-042936_add_rollover_params_table/up.sql
Outdated
Show resolved
Hide resolved
6f84e1a
to
c0d726e
Compare
diesel::result::DatabaseErrorKind::UniqueViolation, | ||
_, | ||
)) => { | ||
tracing::debug!( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this expected to happen? If not, shall we set this to warn
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can happen if we run the task too often and we try to insert the same funding fee event more than once. There is no harm if this does happen, so I think debug
is fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Imho this is good to go. Let's get it in and test without any funding rate until the next release to ensure that nothing broke.
The `Position` model should be as strongly typed as possible. Converting to i64 should happen _just_ before inserting into the database.
This server can be used during local development to query the coordinator database conveniently e.g. ``` $ curl http://localhost:3002/dlc_protocols | jq . [ { "id": 1, "protocol_id": "fb4c6f27-db9d-4b2d-8539-6e609e1559b9", "previous_protocol_id": null, "channel_id": "f689230c5477176961301cb22093cfd52050a52356812039f9c298fe8859eba2", "contract_id": "972c5fcafbd6bbbf874a9a5cd44d9ddd05a2e3f3ae96d5bd771e5af1b8cae1a6", "protocol_state": "Success", "trader_pubkey": "02e9f7a0e0d6eeab989dcbb4e91f0bf95f583b09d8d324cc9635807e844eea82bc", "timestamp": "2024-05-13T05:32:36.452958+00:00", "protocol_type": "open-channel" } ] ``` It can also be used in the e2e tests to easily assert on the state of the coordinator, without needing to add dedicated HTTP API endpoints. It might even be used to modify the coordinator state to create interesting test scenarios.
The coordinator generates funding fee events as funding rates are added to the database and applies them on rollover. The app displays funding fee events in a dedicated `Trades` tab, together with regular trades. The funding fee events are added as soon as the app learns about them from the coordinator, which is usually before they are resolved. Additionally, the app will update the position after rolling over based on the funding fee events that were resolved with the rollover. Missing bits: - Apply funding fee events on closing and resizing. - Considering funding fee events when deciding if positions need to be liquidated. - Add some unit tests. - Expanding e2e tests. - Add fee rate to app's `Trades` tab. - Display next fee rate in app's `Trade` screen. Co-authored-by: Philipp Hoenisch <philipp@hoenisch.at>
We only support one position at a time at the moment. Co-authored-by: Richard Holzeis <richard@holzeis.me>
c0d726e
to
e825e93
Compare
Superseded by #2584. |
Begins to address https://github.com/get10101/meta/issues/330.
This PR does not address the app's side, which amounts to displaying the next funding rate, seeing paid funding rates, etc.