# CW-Hyperlane Yorke Review

<aside>
💡 Reviewed by: Eric / Eddy
Reviewed At: 2023.10.05 (KST)

</aside>

## Abstract

[https://github.com/many-things/cw-hyperlane/pull/38/files](https://github.com/many-things/cw-hyperlane/pull/38/files)

- This Reviewing documents based on above PR

---

# ISM-Multisig

pub use crate::error::ContractError;

// version info for migration info
pub const CONTRACT_NAME: &str = env!("CARGO_PKG_NAME");
pub const CONTRACT_VERSION: &str = env!("CARGO_PKG_VERSION");

// we should probably change this to something else in the agents
const PREFIX: &str = "\x19Ethereum Signed Message:\n";

// can we reuse hyperlane-monorepo/rust/hyperlane-core/src/types/checkpoint ?

> /contracts/ism-multisig/src/lib.rs ( [Link](https://github.com/yorhodes/cw-hyperlane/blob/86173b44d89d73c82f704e0d069afa6c657c36e7/contracts/ism-multisig/src/lib.rs#L18) )
>

### Q. We should probably change this to something else in the agents

Ans: We hopes so. In that case, We have to stop using Ethereum signer if you want to remove that prefix. For now, therefore, We still use this parts.

### Q. Can we reuse `hyperlane-monorepo/rust/hyperlane-core/src/types/checkpoint`?

Ans: We can use that checkpoint type If the hyperlane-core is wasm compatible, and then the other way is copy-and-paste that type into our contract. But we don’t think it is not appliable because of the types in the sturcture.

---

// does this account for signatures.length > threshold?
// does this account for signatures matching ordering but not index of validator set?
let success: u8 = verifiable_cases
    .into_iter()
    .map(|v| {
        deps.api
            .secp256k1_verify(&hashed_message, &v.1[0..64], &v.0)
            .unwrap() as u8
    })
    .sum();

> /contracts/ism-multisig/src/query.rs ( [Link](https://github.com/many-things/cw-hyperlane/blob/86173b44d89d73c82f704e0d069afa6c657c36e7/contracts/ism-multisig/src/query.rs#L54) )
>

### Q. Does this account for signatures.length > threshold?

### Q. Does this account for signatures matching ordering but not index of validator set?

Ans: Both are yes

---

# Mailbox

let config = Config {
    owner: deps.api.addr_validate(&msg.owner)?,
    factory: info.sender,
    default_ism: deps.api.addr_validate(&msg.default_ism)?,
    default_hook: deps.api.addr_validate(&msg.default_hook)?,
};

// does MerkleTree.default() initialize to ZERO_HASHES tree?
MESSAGE_TREE.save(deps.storage, &MerkleTree::default())?;
CONFIG.save(deps.storage, &config)?;
PAUSE.save(deps.storage, &false)?;
NONCE.save(deps.storage, &0)?;

> /contracts/mailbox/src/contract.rs ( [Link](https://github.com/yorhodes/cw-hyperlane/blob/86173b44d89d73c82f704e0d069afa6c657c36e7/contracts/mailbox/src/contract.rs#L24) )
>

### Q. Does `MerkleTree.default()` initialize to ZERO_HASHES tree?

Ans: Yes. `MerkleTree` structure derives `Default` trait, which available to set the whole structure variable as 0.

---

// would rename this assert_undelivered
pub fn assert_already_delivered(storage: &dyn Storage, id: Binary) -> Result<(), ContractError> {
    if DELIVERY.may_load(storage, id.0)?.is_some() {
        return Err(ContractError::AlreadyDeliveredMessage {});
    }

    Ok(())
}

> /contracts/mailbox/src/state.rs ( [Link](https://github.com/many-things/cw-hyperlane/blob/86173b44d89d73c82f704e0d069afa6c657c36e7/contracts/mailbox/src/state.rs#L98C1-L99C1) )
>

### Q. Would rename this assert_undelivered

Ans: Right, I applied this changes. [[ Link ]](https://github.com/many-things/cw-hyperlane/blob/6fda1bfbc1b8b03d806e1cf00c80ca66c492ccbc/contracts/mailbox/src/state.rs#L98-L104)

---

// in ethereum we mark delivered before verifying because if verify reverts, so will the state change
DELIVERY.save(deps.storage, id.0.clone(), &Delivery { ism: ism.clone() })?;

ism_verify(&deps.querier, &ism, metadata, message)?;

let handle_msg = WasmMsg::Execute {
    contract_addr: recipient.to_string(),
    msg: to_binary(&ExpectedHandlerMsg::Handle(HandleMsg {
        origin: decoded_msg.origin_domain,
        sender: decoded_msg.sender.clone().into(),
        body: decoded_msg.body.into(),
    }))?,
    funds: vec![],
};

> /contracts/mailbox/src/core.rs ( [Link](https://github.com/many-things/cw-hyperlane/blob/86173b44d89d73c82f704e0d069afa6c657c36e7/contracts/mailbox/src/core.rs#L109) )
>

### Q. In ethereum, we mark delivered before verifying because if verify reverts, so will the state change

Ans: In CosmWasm too. So we marked delivered before verifying, and there’s no need to care about reentrancy while it uses actor model.

---

// we should discuss default vs required hook here
// is it possible to have the developer provide their own hook?
let binary_msg: HexBinary = msg.clone().into();
let wasm_msg = WasmMsg::Execute {
    contract_addr: config.default_hook.to_string(),
    msg: to_binary(
        &hpl_interface::post_dispatch_hook::PostDispatchMsg::PostDispatch {
            metadata: binary_msg.as_slice()[0..0].into(), // Should be handle without metadata?
            message: binary_msg,
        },
    )?,
    funds: info.funds,
};

> /contracts/mailbox/src/core.rs ( [Link](https://github.com/many-things/cw-hyperlane/blob/86173b44d89d73c82f704e0d069afa6c657c36e7/contracts/mailbox/src/core.rs#L66) )
>

### Q. We should discuss default vs required hook here. is it possible to have the developer provide their own hook?

Ans: User can set their own default hook. However, cannot change hook for each message. We don’t current have the hook routing for specific message.

---

// we only emit these redundantly in the ethereum mailbox because of "indexed" fields
pub fn emit_dispatch(msg: Message) -> Event {
    Event::new("mailbox_dispatch")
        .add_attribute("sender", HexBinary::from(msg.sender.clone()).to_hex())
        .add_attribute("destination", msg.dest_domain.to_string())
        .add_attribute("recipient", HexBinary::from(msg.recipient.clone()).to_hex())
        .add_attribute("message", HexBinary::from(msg).to_hex())
}

> /contracts/mailbox/src/events.rs ( [Link](https://github.com/many-things/cw-hyperlane/blob/86173b44d89d73c82f704e0d069afa6c657c36e7/contracts/mailbox/src/event.rs#L26) )
>

### Q. We only emit these redundantly in the ethereum mailbox because of "indexed" fields.

Ans: We choose events according to solidity implemnetation. However, we can remove those redundant fields because cosmos doesnt have indexed fields.