bekauz commented 9 months ago

problem

currently all clocked contracts are attempting to enqueue themselves to the clock on instantiation by submitting enqueue_msg(clock_addr.as_str())?.

this calls the following method on the clock:

      ExecuteMsg::Enqueue {} => {
          if QUEUE.has(deps.storage, info.sender.clone()) {
              return Err(ContractError::AlreadyEnqueued);
          }
          // Make sure the caller is whitelisted
          if !WHITELIST
              .load(deps.storage)?
              .iter()
              .any(|a| a == info.sender)
          {
              return Err(ContractError::NotWhitelisted);
          }
          // Make sure the caller is a contract
          deps.querier
              .query_wasm_contract_info(info.sender.as_str())
              .map_err(|e| ContractError::NotContract(e.to_string()))?;

          QUEUE.enqueue(deps.storage, info.sender.clone())?;
          Ok(Response::default()
              .add_attribute("method", "execute_enqueue")
              .add_attribute("sender", info.sender))
      }

this whitelist validation works because our top level covenant contracts facilitate the instantiation flow by using instantiate2.

however, this limits the extent of how we can use our contracts without the top level covenant. consider we want to use a liquid pooler without the full covenant flow:

to instantiate the liquid pooler, we need to have a clock that has the liquid pooler whitelisted
to whitelist the liquid pooler on the clock we need to know its address

possible solutions

possible solution 1: optional clock

the clock could be made optional on all contracts that attempt to enqueue themselves on instantiation.

if the clock address is passed along with the message, nothing changes. we validate the address, save it, and attempt to enqueue ourselves which will trigger the usual whitelist validation.

if no clock address is passed in, the contracts do not save the clock and do not attempt to enqueue themselves.

in both cases, the incoming tick validation would change slightly: if clock address is available, we validate as usual. otherwise we skip the validation.

this would also affect the top level covenants design:

if clock is enabled, nothing changes other than passing Some(clock_addr) instead of just clock_addr
if clock is disabled, we skip the clock instantiation altogether and pass None for the clock address to clocked components.

possible solution 2: clock contract changes

the clock could perform a no-op in case of a non-whitelisted contract attempting to enqueue itself. this way, the enqueueing contract would not receive an error from the clock and instantiate fine.

after all contracts are instantiated and their addresses are known, we would update the clock whitelist/queue by migrating it.

this could be done by adding additional configuration to our current clock, or by introducing new types of clocks (which is probably a good long term idea anyways).

possible solution 3: manually querying the predictable instantiate2 address

we could manually query the predictable instantiate2 build address (query wasm build-address) for each contract that would be attempting to enqueue itself. those addresses could then be whitelisted on the clock, and we could proceed with regular instantiate2'ion of the contracts.

haven't tested this yet but seems possible. no code changes would be needed, but a little manual work would be needed for each instantiation flow.

possible solution 4: making a generic top level covenant contract

we could try to come up with a generic top level covenant design to address such cases.

this would require the most work from all possible solutions.

covenant instantiation would likely take in a vector of custom instantiation messages that would wrap around specific contract instantiation messages, similar to how we used the PresetInstantiateFields in v1 covenant.

uditvira commented 8 months ago

Thank you for putting together the problem and possible solutions!

I have a strong preference for option 1. This option provides the most flexibility in the use of each component, with or without instantiating a clock. It also allows for components to choose whether ticking needs to be mediated by a dedicated clock or not.

For option 2, is there a way to receive the error from the clock and not fail instantiation on the error? It should just handle the error appropriately. I don't like the idea of making it work by removing feedback from the clock.

Option 3 is a work around we can use currently and requires no architectural changes. However it offers poor UX for users of our contracts.

Option 4 seems complex to architect and implement. It will require us to make components a lot more generic. I'm not in favour of doing this currently.

Also unless I'm mistaken none of the options actually preclude each other, meaning we can adopt multiple simultaneously

bekauz commented 8 months ago

That makes sense. I will spec out options 1 & 2 together, as indeed they are both nice to have.

timewave-computer / covenants

enable covenant components to be used without the top level covenant #165