refactor instantiation logic #94
Conversation
SebastianElvis
commented
Dec 6, 2024
•
SebastianElvis
commented
- remove contract addr in config
- remove cascaded instantiation logic
- use custom query to get contract addresses
- unit tests
- e2e tests in Babylon SDK query: custom query for params babylon-sdk#77
packages/bindings/src/babylon_sdk.rs
Outdated
| pub fn get_babylon_sdk_params(querier: &QuerierWrapper) -> Result<Params, StdError> { | ||
| let params = querier.query_grpc(QUERY_PARAMS_PATH.to_owned(), Binary::new("".into()))?; | ||
| let params = QueryParamsResponse::from(params).params; | ||
| Ok(params) | ||
| } |
There was a problem hiding this comment.
I think this approach is not without merit, but it would be much simpler IMO to just pass the needed params during instantiation. The contracts have a clear dependencies, so it's just a matter of instantiating them in the right order and pass required parameters (other contracts' addresses) between them at instantiation.
There was a problem hiding this comment.
Finally got back to this PR. In the ideal case, indeed it's better if we let each contract to only keep references of contracts that it interacts with. However, we have a couple of cyclic dependencies between contracts, making it difficult for instantiating them in order.
In addition, the contract might not be persistent. It's possible that the consumer's gov prop deploys another sets of contracts and specifies them as the "official version" through the governance proposal. Once this happens then there can be discrepencies between parameters of Babylon SDK and Babylon contracts.
Thus I proposed we make contract addresses in Babylon SDK module parameters as single source of truth, and let contract find each other through Babylon SDK. What do you think?
| pub babylon: Addr, | ||
| pub staking: Addr, |
There was a problem hiding this comment.
I would just keep these, and pass / set them during instantiation.
| pub fn get_activated_height(querier: &QuerierWrapper) -> StdResult<u64> { | ||
| let params = get_babylon_sdk_params(querier)?; |
There was a problem hiding this comment.
Nit: This is good for encapsulation, but bad for performance. In any case, since you already have it why not pass the entire Config by reference instead?
