-
Notifications
You must be signed in to change notification settings - Fork 13
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
Issue 156 queue reserve inits #158
Changes from 1 commit
5d58cd4
6539029
a273111
1cc7ee3
a3597ff
692aca8
4823ca0
ace67dd
685cd26
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,3 +8,6 @@ pub const SCALAR_7: i128 = 1_0000000; | |
|
||
// seconds per year | ||
pub const SECONDS_PER_YEAR: i128 = 31536000; | ||
|
||
// approximate week in blocks assuming 5 seconds per block | ||
pub const WEEK_IN_BLOCKS: u32 = 120960; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A week seems like a really long time, no? Consider a DAO doing this:
So its a 15 day, 2 proposal process to add a reserve to the pool, assuming they are smart enough to double dip the timelock and proposal period. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will most DAO's have a mandatory 2 day period before voting? I made initialization permissionless so I think it's important the queue time is longer than a typical DAO vote -> execution time so they can cancel initializations if a malicious one snuck in without anyone noticing |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,9 @@ | ||
use crate::{ | ||
auctions::{self, AuctionData}, | ||
constants::WEEK_IN_BLOCKS, | ||
emissions::{self, ReserveEmissionMetadata}, | ||
pool::{self, Positions, Request}, | ||
storage::{self, ReserveConfig}, | ||
storage::{self, QueuedReserveInit, ReserveConfig}, | ||
}; | ||
use soroban_sdk::{contract, contractclient, contractimpl, Address, Env, Symbol, Vec}; | ||
|
||
|
@@ -37,6 +38,7 @@ pub trait Pool { | |
backstop_id: Address, | ||
blnd_id: Address, | ||
usdc_id: Address, | ||
reserves: Vec<(Address, ReserveConfig)>, | ||
); | ||
|
||
/// (Admin only) Set a new address as the admin of this pool | ||
|
@@ -57,15 +59,35 @@ pub trait Pool { | |
/// If the caller is not the admin | ||
fn update_pool(e: Env, backstop_take_rate: u64); | ||
|
||
/// (Admin only) Initialize a reserve in the pool | ||
/// (Admin only) Queues the initialization of a reserve in the pool | ||
/// | ||
/// ### Arguments | ||
/// * `asset` - The underlying asset to add as a reserve | ||
/// * `config` - The ReserveConfig for the reserve | ||
/// | ||
/// ### Panics | ||
/// If the caller is not the admin or the reserve is already setup | ||
fn init_reserve(e: Env, asset: Address, metadata: ReserveConfig) -> u32; | ||
/// If the caller is not the admin | ||
fn queue_init_reserve(e: Env, asset: Address, metadata: ReserveConfig); | ||
|
||
/// (Admin only) Cancels the queued initialization of a reserve in the pool | ||
/// | ||
/// ### Arguments | ||
/// * `asset` - The underlying asset to add as a reserve | ||
/// | ||
/// ### Panics | ||
/// If the caller is not the admin or the reserve is not queued for initialization | ||
fn cancel_init_reserve(e: Env, asset: Address); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is this necessary? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it's potentially problematic not to have one - consider someone sneaks through a bad DAO proposal or tricks a multisig into adding a reserve - note that executing the queued initialization is not permissioned |
||
|
||
/// (Admin only) Executes the queued initialization of a reserve in the pool | ||
/// | ||
/// ### Arguments | ||
/// * `asset` - The underlying asset to add as a reserve | ||
/// | ||
/// ### Panics | ||
/// If the reserve is not queued for initialization | ||
/// or is already setup | ||
/// or has invalid metadata | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we should validate the metadata on queue There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That duplicates code since we reuse the initialize function which validates metadata |
||
fn init_reserve(e: Env, asset: Address) -> u32; | ||
|
||
/// (Admin only) Update a reserve in the pool | ||
/// | ||
|
@@ -224,6 +246,7 @@ impl Pool for PoolContract { | |
backstop_id: Address, | ||
blnd_id: Address, | ||
usdc_id: Address, | ||
reserves: Vec<(Address, ReserveConfig)>, | ||
) { | ||
storage::extend_instance(&e); | ||
|
||
|
@@ -236,6 +259,7 @@ impl Pool for PoolContract { | |
&backstop_id, | ||
&blnd_id, | ||
&usdc_id, | ||
&reserves, | ||
); | ||
} | ||
|
||
|
@@ -261,15 +285,39 @@ impl Pool for PoolContract { | |
.publish((Symbol::new(&e, "update_pool"), admin), backstop_take_rate); | ||
} | ||
|
||
fn init_reserve(e: Env, asset: Address, config: ReserveConfig) -> u32 { | ||
fn queue_init_reserve(e: Env, asset: Address, metadata: ReserveConfig) { | ||
storage::extend_instance(&e); | ||
let admin = storage::get_admin(&e); | ||
admin.require_auth(); | ||
|
||
let index = pool::initialize_reserve(&e, &asset, &config); | ||
storage::set_queued_reserve_init( | ||
&e, | ||
&QueuedReserveInit { | ||
new_config: metadata.clone(), | ||
unlock_block: e.ledger().sequence() + WEEK_IN_BLOCKS, | ||
}, | ||
&asset, | ||
); | ||
|
||
e.events().publish( | ||
(Symbol::new(&e, "queue_init_reserve"), admin), | ||
(asset, metadata), | ||
); | ||
} | ||
|
||
fn cancel_init_reserve(e: Env, asset: Address) { | ||
storage::extend_instance(&e); | ||
let admin = storage::get_admin(&e); | ||
admin.require_auth(); | ||
|
||
storage::del_queued_reserve_init(&e, &asset); | ||
|
||
e.events() | ||
.publish((Symbol::new(&e, "init_reserve"), admin), (asset, index)); | ||
.publish((Symbol::new(&e, "cancel_init_reserve"), admin), asset); | ||
} | ||
|
||
fn init_reserve(e: Env, asset: Address) -> u32 { | ||
let index = pool::execute_queued_reserve_initialization(&e, &asset); | ||
index | ||
} | ||
|
||
|
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 be fetched from the Pool WASM, to avoid duplication. The Pool Factory is already dependent on it.