Skip to content
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

HookModule update cannot handle address strings #5456

Open
ltyu opened this issue Feb 12, 2025 · 1 comment
Open

HookModule update cannot handle address strings #5456

ltyu opened this issue Feb 12, 2025 · 1 comment
Milestone

Comments

@ltyu
Copy link
Contributor

ltyu commented Feb 12, 2025

Context

Renzo specifies an aggregate of the defaultHook with their ProtocolFeel. The config specifies an address for the default hook.

hook:
    hooks:
      - "0x9e8fFb1c26099e75Dd5D794030e2E9AA51471c25"
      - beneficiary: "0x0e60fd361fF5b90088e1782e6b21A7D177d462C5"
        maxProtocolFee: "100000000000000000000"
        owner: "0x0e60fd361fF5b90088e1782e6b21A7D177d462C5"
        protocolFee: "158365200000000"
        type: protocolFee
    type: aggregationHook

Problem

When running warp apply and subsequently HookModule.update(), the Module attempts to redeploy the hook, despite no changes. This is due to the defaultHook address being compared to the derived version of it.

Additional Context

In updateExistingHook() of EvmERC20WarpModule, the EvmHookModule is initialized with actualConfig.hook

const hookModule = new EvmHookModule(
this.multiProvider,
{
chain: this.args.chain,
config: actualConfig.hook,

Subsequently hookModule.update() is called with expectedConfig.hook, which get compared with a derived actual hookConfig (via this.read())

// We need to normalize the current and target configs to compare.
const normalizedCurrentConfig = normalizeConfig(await this.read());
const normalizedTargetConfig = normalizeConfig(targetConfig);
// If configs match, no updates needed
if (deepEquals(normalizedCurrentConfig, normalizedTargetConfig)) {
return [];
}
if (
this.shouldDeployNewHook(normalizedCurrentConfig, normalizedTargetConfig)
) {
const contract = await this.deploy({
config: normalizedTargetConfig,
});

The result will always result in a diff, because as we recall, the expected hook config contains an address (e.g. 0x9e8fFb1c26099e75Dd5D794030e2E9AA51471c25), and thus deploy a new hook.

Solution

  • Consider adding a short circuit such that if a hook address is supplied, derive it to compare
  • Alternatively, compare the address to the actual address
  • Required: Add a unit test in the respective Module test
@ltyu ltyu changed the title HookModule update cannot handle addresses HookModule update cannot handle address strings Feb 12, 2025
@nambrot
Copy link
Contributor

nambrot commented Feb 12, 2025

This probably should just use @yorhodes 's new DefaultFaullbackRoutingHook #5405, wonder if that would fix that?

@nambrot nambrot moved this to Sprint in Hyperlane Tasks Feb 12, 2025
@nambrot nambrot added this to the Warp Speed milestone Feb 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Sprint
Development

No branches or pull requests

3 participants