terra-money / warp-contracts

Other
15 stars 14 forks source link

Exploring asset lock or asset isolation on pending jobs #38

Closed dev8723 closed 1 year ago

dev8723 commented 1 year ago

Implementing asset lock on pending jobs

We used to call this fund locker, fund reservation, asset locker, they are all same idea.

Problem statement

Think of this scenario, a user place a limit order on astroport, swapping 100 USDC to 200 LUNA, this order will be executed when LUNA/USDC price reach 0.5. Under the hood, user will deposit the 100 USDC (the offered token) to its Warp account, along with the eviction fee (without eviction fee, order will expire after 24 hours). However, user can withdraw any token they have in their Warp account, currently there's no check whether those tokens are used for a pending job. So if a user goes to Warp web app, it can withdraw that 100 USDC without any warning, leading to the pending limit order to fail.

This is a common problem for any Warp jobs that needs token to spend. (e.g. limit order, DCA order, payment)

Proposed solution

Solution 1 virtual lock

When a job is created, creator will specify the assets and their amount should be locked for the job, when a user tries to withdraw assets, we check the lock on all pending jobs.

Detail implementation

Considering this is a complex feature with many corner cases, we should break it down to pieces.

  1. As the first step, we add a new field to job struct, call it assets_to_lock, it's type will be similar to assets_to_withdraw's type: Option<Vec<AssetInfo>>, but we need to modify it to support specifying an amount for each asset.

    • This is a very easy and backward compatible change, once implemented, creator can specify the assets to lock, and all frontend that supports withdraw assets from Warp account, can check this field to calculate what assets cannot be withdraw. This is not perfect, as users could have old jobs without this field, but it's a good starting point. Especially we don't have much job on mainnet (about 160 jobs, pending and finished as of 07/10/2023).

    • For recurring job, we need to update assets_to_lock after job executed successfully or failed accordingly. Maybe we need a more sophisticated way to represent assets_to_lock for recurring job, like break it down to how much asset for future job reward, how much for eviction, how much for actual offering and amount for single execution. So it's easier to update after each execution.

  2. Once we have the asset lock info on job, we can add 2 endpoints to Warp account, one is to query the assets that are available to withdraw, the other is to withdraw from those asset. Essentially we are moving the frontend calculation logic from step 1 on-chain.

    • The challenge here is how do we calculate it in an efficient way, it's easy to do on the frontend since we can use indexer and just more compute power off-chain. Currently all jobs are stored in the controller contract in an indexed map, key is job id, index key is job reward. We need a way to efficiently get all jobs by owner address, with current data structure, we need to loop over all pending jobs, it's O(N) complexity.

    • However, even if we end up just query the existing job map, I think it might be acceptable, since we don't expect people to have a lot of pending jobs. But if it's a contract owned Warp account, it's possible for it to have lots of pending jobs.

    • I can't think of any solution here, maybe we create another index map or add owner address as another index to existing map? Not sure if it's possible, we need more discussion here.

  3. This is an optional step, I'm thinking maybe we should disable Warp account to execute arbitrary msg, at least disable it from executing arbitrary withdraw asset msg, so user can only withdraw through the endpoint in step 2, which checks the asset lock.

  4. This is another optional change, I'm not sure if it's a good idea. But in order to solve the query complexity issue above, we add a field to Warp account, call it total_assets_to_lock, it sums all the assets to lock from all pending jobs.

    • pro: when user wants to withdraw, we only need to check this field, no need to look at pending jobs anymore.
    • con: we need to keep this field in sync with all the pending jobs, meaning when a job is created, we need to update it, when a job is cancelled / executed / evicted, we also need to update it, that could be a lot of work, but it avoids having any gas heavy operation (query all pending jobs).

Solution 2 job based account

Make Warp account per job, essentially eliminating the concept of Warp account, so token used for the job will be naturally isolated from each other. I believe this is how clockwork works. Instead of creating an account for every job created, we can give user the option to whether create a job account or not, ideally for jobs that need to spend assets, user will create job account.

dev8723 commented 1 year ago

I think it's really difficult to express the locked asset in job struct because:

  1. Recurring job: when a recurring job is executed, we need to find how much asset is used for single execution and update the assets_to_lock on the follow up job created. (e.g. DCA job runs 10 times, each time spend 1 LUNA, first job assets_to_lock will be 10 LUNA, second job assets_to_lock will be 9 LUNA. So we need another field say assets_for_single_execution in job struct and set it to 1 LUNA. This update logic can be infinitely complex due to below.
  2. assets_to_lock is not static: Above is the simple case because the value is static (1 LUNA is known at the creation of the job), what if value is dynamic, like a variable fetched from the query on the fly? That makes it infinitely complex to implement as we need to extract the token and amount from var which could be anything. This non-static issue applies to job only executes once as well.

Due to reasons above, I suggest we reconsider the 1 job 1 account approach, we add an optional boolean flag create_job_account to CreateJobMsg.

  1. If it's set to true, we will create a warp account only for the job. So for jobs that need to spend assets (like limit order or DCA or many defi strategies), we give people the option to create warp account only for that job, in this way, funds used for that job will be isolated by nature.
  2. If unset or false, we use the default warp account, think of this as a permanent account used for jobs that don't need to spend asset (e.g. claim staking reward, execute gov proposal).
dev8723 commented 1 year ago

Even we go with 1 job 1 account approach for jobs that spend assets, we still need an optional asset_lock on job struct for the regular account (or we can call it primary account) to lock the protocol fee paid for eviction and reward. But this is trivial to implement.

dev8723 commented 1 year ago

Latest finding, it's really tricky to get job account address when we want to create job account and job in 1 tx, i tried 2 approach

  1. having a special variable $warp.account in msg / var / condition, user don't need to set it explicitly but they can just use it in msg / var / condition, it will be replaced with real value as soon as job is created (because at this time job account is also created so we know the address), however i found it challenging to replace it as it could be in some encoded part of msg / var, but this is still an elegant solution in the long run i think, it would be handy to provide some system vars like this.
  2. this is a very hacky solution, i use a regular var for job account address, but i won't fill it when calling the tx because job account is not created at that time, instead i add another function to resolver to set this variable in reply, this approach is easy to implement but too hacky.
dev8723 commented 1 year ago

While im thinking about above 2 approaches, i realize what if we give up creating job account and job in 1 tx? but we still don't want users to sign 2 tx (tx1 create job account, tx2 create job). i think one workaround is changing the way we think about job account, so i propose sub account to replace job account, sub account is similar to job account, user can create unlimited sub accounts, sub account is also used with jobs that spend token like limit order, but the difference is it will be created beforehand, say users first comes to warp or astroport and when they create the default warp account, we also create 10 sub accounts, we store sub accounts info under default account, and track which one is in use (associated with a pending job), so it's more reusable and as long as users don't have more than 10 pending orders at the same time, they don't need to create new sub account when creating limit order (i.e. sign 2 tx), so most of time users are still signing just 1 tx.

i really like the sub account idea as people can also use it to do different kind of jobs, similar to sub account function at many exchanges. the only downside of it is users are more aware of warp now, which might hurt ux if apps want to abstract warp away.

Ooooo, actually we can still abstract sub account, when a user is creating a limit order using the last available sub account, we will create say 5 more sub accounts in the same tx for future usage, we could also create sub account lazily, i.e. create 1 sub account when default account is created, then only create a new one when a job is created.

dev8723 commented 1 year ago

Where should we store the in use sub accounts and free sub accounts? i think there are 2 places

  1. store them in the primary account
    • pro: keep controller state small
    • con: in case we need to upgrade (migrate) warp account contract, we need to do the migration for all primary accounts, which could be a lot, but this might not be an issue.
  2. store them in controller
    • pro: easier to query and update, since we need to update it when a job is created / executed / deleted / evicted, keeping them all in controller can avoid cross contract call to warp account, so should save us more gas.
    • con: controller state will be so big, unlike job which we can prune the finished ones after a while, i think it's better to not prune sub accounts since they can hold assets.
dev8723 commented 1 year ago

we ended up with job account approach, target release by end of 2023