Skip to content

Bug: encodeTransactions desyncs targets/values/calldatas when a case throws mid-push #70

Description

@r4topunk

Summary

encodeTransactions in src/lib/proposal-utils.ts pushes into targets before it calls parseEther / encodeFunctionData. If the encoding throws, the catch block at the bottom of the loop pushes placeholders into all three arrays — leaving targets one element longer than values and calldatas. A subsequent valid transaction then encodes into mismatched indices.

This is a real governance risk: a malformed user input in one row of the propose wizard would silently submit a corrupted (targets, values, calldatas) triplet to the Governor.

Repro

encodeTransactions([
  { type: "send-eth", target: "not-an-address", value: "not-a-number" },
  { type: "send-eth", target: "0x…dEaD", value: "1" },
]);

Current output:

targets.length   === 3   // ["not-an-address", "0x", "0x…dEaD"]
values.length    === 2   // [0n, parseEther("1")]
calldatas.length === 2   // ["0x", "0x"]

Expected: all three arrays length 2, with index 0 being the all-placeholder entry and index 1 the correctly encoded ETH transfer.

Source

src/lib/proposal-utils.ts#L12-L17 — the send-eth case (and several others) pushes to targets on a line that can't throw, then calls parseEther on the next line, which can throw. The same pattern exists for send-usdc, send-tokens, send-nfts, custom, and droposal — any of them can partially push before an inner call throws.

Fix sketch

Build the three values in local variables first, then push all three only after the encoding succeeds. Something like:

case "send-eth": {
  const target = tx.target as `0x${string}`;
  const value = parseEther(tx.value || "0");
  targets.push(target);
  values.push(value);
  calldatas.push("0x");
  break;
}

Apply the same shape to every case so the catch is the sole writer of placeholder rows.

Tracking

Surfaced by the unit tests added in #69. Currently marked it.todo("should keep targets/values/calldatas arrays in sync when a case throws mid-push") in src/lib/proposal-utils.test.ts — flip it to a real it(...) when fixed.

Generated with Claude Code

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    • Status
      Backlog

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions