Feat/add ibc v2 to node - #4
Open
aluque-peersyst wants to merge 62 commits into
Open
Conversation
ibctesting cannot construct this app: setupWithGenesisValSet rebuilds bank
genesis with an empty metadata list and x/vm.InitGenesis then panics with
"denom metadata acbdc could not be found". NewIBCCoordinator lets the existing
integration network build genesis instead and fills in the TestChain fields
ibctesting needs.
Four obstacles it resolves, each of which would otherwise stop the first
person who tries:
- bank genesis metadata wiped by upstream setup
- GetIBCChain leaving SenderAccount/SenderAccounts/ProposedHeader unset
- chain ids not configurable (adds WithChainID; the EVM chain id is parsed
out of the cosmos chain id string, so two chains need two ids)
- the harness init clock running ahead of ibctesting's epoch, moving time
backwards on the first block and failing client creation
SetupSdkConfig is made idempotent because it ends in config.Seal(), so the
second test in a binary panicked with "Config is sealed".
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The transfer keeper was constructed with ChannelKeeper as its ICS4Wrapper, so v1 sends went straight to core IBC and outbound quotas never applied. Inbound was limited; outbound was not. RateLimitKeeper itself implements ICS4Wrapper, so the fix is one WithICS4Wrapper call after construction. Verified that ics4Wrapper.SendPacket is only reached from transferV1Packet, so there is no double counting with ratelimitv2, whose accounting happens in OnSendPacket. WARNING: this changes v1 consensus behaviour. Outbound transfers become quota-enforced at the upgrade height. Genesis ships rate_limits: [] and no bootstrap script provisions any, so it should be inert on day one, but confirm with 'q ratelimit list-rate-limits' against the live node before shipping: if quotas exist they were tuned for inbound only. Kept as its own commit for that reason, and it needs its own line in the upgrade notes. Pre-existing and unrelated to IBC v2. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
x/cbdc's issuance_paused is checked only in the module's own mint and burn
path. It does not touch the transfer module, IBC, or the bank send path, so
pausing issuance does not stop a corridor: acbdc keeps flowing out and vouchers
keep flowing in. For an incident where the corridor itself is the problem, the
switch everyone would reach for does nothing.
paused_ibc_clients is a new x/cbdc param, edited by governance through the
existing MsgUpdateParams and sitting beside issuance_paused so there is one
place to look during an incident. corridorpause is wired outermost in the v2
transfer stack, so a paused client is refused before anything escrows, mints or
converts.
- Per-corridor, not global: with N countries, pausing every corridor because
one counterparty is in trouble is an outage, not an incident response.
- Receives return a failure acknowledgement rather than an error, so the ack
travels back and refunds the counterparty's sender. An error would strand
the packet until timeout with their funds locked.
- Timeouts and acknowledgements are deliberately not gated: both settle
transfers that already happened, and blocking them would strand exactly the
funds the pause exists to protect.
- Events on both paths, so a pause is visible in the block stream and not
only as a failed tx.
ratelimitv2guard is added alongside because ratelimitv2 silently passes v2
sends that have no client-keyed quota. The guard makes that gap loud without
blocking the packet, so a deployment that forgets client-keyed quotas cannot
leave v2 outflows unlimited unnoticed.
Params validation rejects empty and duplicate entries, since a proposal must
not read as pausing something it does not.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A native in-tree light client, so the corridor's trust statement is
cryptographic verification of the counterparty's own consensus rather than a
quorum of attestors. exported.LightClientModule is a plain interface and
Router.AddRoute accepts any implementation, so this needs no ibc-go change and
no move to v11.
Layered so the audited part survives a dependency bump: x/qbftclient/types
holds the verification core and does not import ibc-go; x/qbftclient is a thin
adapter implementing the thirteen LightClientModule methods.
- Validator set is followed through the header chain rather than pinned at
creation, so a rotation is a client implementation detail rather than a
client migration (which would mean a new escrow address and voucher denom).
- Misbehaviour freeze ships now: without it the client accepts whichever fork
it is shown first.
- RecoverClient is implemented, not stubbed. A freeze with no recovery is
terminal, and it checks the substitute describes the same counterparty --
same chain id, same IBC contract, greater height -- so recovery cannot
silently repoint a corridor while keeping the escrow address.
Two findings pinned by tests:
- Quorum is ceil(2n/3), taken from Besu's BftHelpers rather than inferred.
- Storage values come back RLP-trimmed, so a 32-byte commitment returns
shorter whenever it has leading zeroes. VerifyCommitment left-pads
internally; a raw comparison would mismatch about one time in 256, and only
in production.
Heights are plain uint64 (QBFT has no revision concept) and Header carries RLP
blobs rather than decomposed fields, because the block hash is taken over that
encoding and re-serialising from parsed fields risks a digest that diverges
from Besu's.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The proof constructor shares the client's verification core, so the relayer
serialises exactly what the client deserialises and the two cannot diverge.
Nothing here signs: every tool emits unsigned transactions for the operator's
own tooling, which keeps key custody out of the relaying path.
- prover/besu reads a real Besu node over JSON-RPC; prover/cosmos produces
the ICS-23 proofs for the return leg (cmd/v2relay is refactored onto it in
a later commit).
- UpdateChain assembles the header chain needed to cross a validator-set
change. Besu does not confine set changes to epoch boundaries -- an epoch
block discards outstanding votes, it is not where changes take effect -- so
the relayer cannot skip one. It binary-searches for change points rather
than walking block by block.
- qbftinit emits an unsigned MsgCreateClient and prints what is being
trusted, warning when the set is below the BFT threshold. The initial
trusted state is the one input a light client cannot verify, so it belongs
in front of the operator rather than buried in a JSON blob.
- qbftrelay is one-shot: client updates plus a packet proof, no event loop,
no retries, no state.
besu.HeaderByNumber re-hashes every reassembled header with our own QBFT
block-hash implementation and compares it against the hash the node reported.
JSON-RPC serves headers as fields, not RLP, so a field the node populates and
go-ethereum ignores would yield a header that decodes cleanly and verifies
against nothing. This stops that at the RPC boundary instead of as an
unexplainable on-chain signature failure.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
v2relay builds a MsgRecvPacket with a commitment proof and emits it unsigned. It is a proof of capability and a manual break-glass tool, not a production relayer: one packet on demand, no event loop, no ack leg, no retries. scripts/ibcv2-devnet stands up two cbdc-node chains plus an ibc-go simapp on one command, which is what closed the gap nothing in this repo could test: a real relayer moving real packets. Two settings there are not optional and are baked into up.sh -- TZ=UTC, because the relayer writes packet deadlines as local wall-clock into a timestamp-without-time-zone column, and pinning the relayer API off port 9000, which proof-api also binds undocumented. local-node.sh gains an UNBONDING_TIME override. The 60s default caps a tendermint client's trusting period below 60s, so clients expire almost immediately and the stock localnet cannot hold an IBC client at all. The default is unchanged; IBC work runs as UNBONDING_TIME=1814400s ./local-node.sh. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Tracks upstream cosmos/evm v0.6.1, which upstream describes as containing "important security fixes ... we recommend all chains upgrade to this patch release as soon as possible using a coordinated upgrade. This release is state breaking." Its changelog includes chore(erc20/v2): align ack validation with ibc-go, in the middleware the v2 transfer stack depends on. Verified against the module proxy before taking it: v0.6.1-xrplevm.1 is a true drop-in. Go 1.23.8, cosmos-sdk v0.53.6, CometBFT v0.38.21, cosmossdk.io/store v1.1.2 and the same ibc-go pseudo-version this chain already builds against -- every pin identical to v0.6.0-xrplevm.6. Do NOT take v1.0.0-rc*, which the fork line now also carries. Despite the version number it is a downgrade on every axis that matters: cosmos-sdk v0.53.0, CometBFT v0.38.17 and ibc-go v10.0.0-beta. It is a trap for anyone scanning the tag list. State breaking costs nothing here because the chain is not deployed. After genesis this becomes a coordinated upgrade, which is the argument for taking it now. go mod tidy promotes cosmos/ics23/go to a direct dependency (the ICS-23 proof constructor in x/qbftclient/prover/cosmos uses it) and drops indirects that are no longer reachable. Verified: go build ./... clean; x/qbftclient, x/poa and x/cbdc unit suites green; tests/integration green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both live suites hardcoded chainId 1338 -- the spoke the tests were first written against. Every Scenario A spoke is the same chain with a different chainId injected by the toolkit, so the hardcoded value silently restricted the tests to one country: against any other spoke signing fails with "Wrong chainId", and in the integration test the ClientState would additionally have described the wrong counterparty. Both now read eth_chainId from the node under test. Verified against a fresh Besu 25.8.0 QBFT chain built to the Scenario A genesis shape with chainId 1337 (brazil): all seven prover/besu live checks pass, and TestLive_InboundLegEndToEnd completes -- a real packet commitment in real Besu storage, real headers accepted by cbdc-node's real ClientKeeper, and a real Merkle-Patricia proof accepted by VerifyMembership. This matters for the N-country goal specifically: the corridor is cbdc-node connected to every Scenario A spoke, so anything keyed to a single chainId cannot be part of proving it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ClientType was "qbft", so ibc-go generated the client id "qbft-0". IBC v2 carries
client ids in the position v1 used for channel ids, and a packet's source and
destination ids are validated with host.ChannelIdentifierValidator, which
requires 8-64 characters. Six is not enough.
MsgCreateClient does not apply that validator, so the failure is silent in
exactly the worst way: clients are created successfully, the client reports
Active, membership proofs verify, and then every single MsgRecvPacket fails with
invalid destination ID: identifier qbft-0 has invalid length: 6,
must be between 8-64 characters
There is no in-place correction. The client id is baked into the counterparty
registration on both sides -- and RegisterCounterparty deletes the client creator,
so it cannot be re-registered -- and under DEC-5 it also determines the escrow
address and the voucher denom. Recovery means a new client pair and a drain.
ClientType is now "qbftclient", giving "qbftclient-0".
Found on a local two-chain rig: a real Besu 25.8.0 QBFT chain carrying the
deployed solidity-ibc-eureka stack, and cbdc-node. Every earlier test passed
because none of them drove a real MsgRecvPacket through the msg server -- the
live test called ClientKeeper.VerifyMembership directly, which does not validate
identifiers. Guarded now by asserting the generated id against
ChannelIdentifierValidator, so the gap cannot reopen.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two pieces of tooling the repo lacked, both needed to drive a real corridor.
scripts/besu-devnet/up.sh brings up a single-validator Besu 25.8.0 QBFT chain
matching the Scenario A spoke genesis. The live tests have always said "bring one
up with the cbweb3 genesis shape" without a way to do it. Genesis constants come
from renderQBFTConfig in cbweb3-platform's toolkit, which is the authoritative
source -- note that provisioning/templates/.../examples/qbftConfigFile.json is
materially stale against it (London only, no zeroBaseFee, empty alloc, zeroed
mixHash), so following that file yields a different chain.
Two things it encodes that cost time to discover:
- --bonsai-historical-block-limit cannot be raised on its own. Besu refuses to
start unless --bonsai-trie-logs-pruning-window-size exceeds it, and the
deployment-parameters doc tells operators to raise only the former.
- generate-blockchain-config --to pointed at a bind mount fails Besu's own
"Output directory already exists" check. It has to write inside the container
and be copied out.
cmd/packetconv translates a solidity-ibc-eureka SendPacket event into the
protobuf channeltypesv2.Packet that MsgRecvPacket carries, and cross-checks the
commitment. That check is the point: the commitment in Besu storage is computed
by ICS24Host, the one cbdc-node verifies the proof against by
channeltypesv2.CommitPacket. They agree byte for byte -- confirmed on real
packets -- and if they ever stop agreeing nothing else in the stack would say why.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…urvive An EVM-native asset crossing ICS-20 arrives as its contract address, because the protocol carries only the denom string -- name, symbol and decimals are lost. ibc-go then synthesises metadata from what it has, with a single denom unit at exponent 0, so a token with 18 decimals on the spoke rendered 777 as 777000000000000000000 on cbdc-node. The erc20 middleware's auto-registered precompile inherited it, making MetaMask and every EVM tool wrong as well. There is no way to correct it after the fact: x/bank has no MsgSetDenomMetadata and neither does x/erc20. But ibc-go's receive path only writes metadata when none exists, and the voucher denom is deterministic -- sha256 of transfer/<client-id>/0x<lowercase address> -- so it can be seeded before any packet moves. seed-voucher-metadata.sh does the derivation and patches genesis; local-node.sh gains an optional SEED_VOUCHER hook following the UNBONDING_TIME precedent, so the default is unchanged. Verified end to end on the bench rig. Before: display transfer/qbftclient-0/0xfe0b7ee2..., symbol 0XFE0B7EE2..., decimals 0. After a second real transfer with metadata seeded: display and symbol tCeBM_BRL, name "Test CeBM BRL", and the precompile reports decimals 18. The derivation was checked against reality rather than trusted -- the script's hash for the live trace matches the denom the chain actually minted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
msgs.Timeout and prover.PacketReceiptProof both existed and neither had ever
been exercised. qbftrelay gains a mode that uses them: instead of proving a
packet commitment is PRESENT on the counterparty, it proves the packet receipt
is ABSENT, which is what entitles this chain to refund its own escrow.
The receipt is keyed by the packet's DESTINATION client, not its source, because
that is the store it would have been written into. Getting that backwards
produces a proof of the wrong slot that verifies correctly and means nothing.
Flag is -as-timeout, not -timeout: the testing package registers -timeout as a
duration in any binary that links it, and this one does transitively via rapid.
The collision surfaced as a usage dump rather than an error.
Proven on the bench rig, cbdc-node -> Besu(Brazil):
1. cbdc-node sent an outbound v2 packet with a 30s absolute timeout,
escrowing 1,000,000 axrp (total-escrow confirmed).
2. It was deliberately never relayed.
3. Waited for a Besu block timestamped past the deadline -- ibc-go requires the
counterparty consensus state at the proof height to be at or after the
packet timeout, so the proof height is bounded from below by wall clock, not
just by the client's progress.
4. Built the MsgTimeout with a real QBFT non-membership proof of the receipt
slot in Besu storage.
5. cbdc-node's real msg server accepted it: events timeout, timeout_packet,
transfer.
6. total-escrow axrp back to 0, and the packet commitment is deleted.
Only the OUTBOUND timeout is provable today. An inbound packet timing out would
be timed out on Besu, verified by AttestationLightClient, which needs an attestor
that does not exist yet -- the same gap that blocks the return and ack legs.
This is the second of DEC-3's four templatisation requirements.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… end
packetconv gains --to-solidity: given a protobuf packet, emit the Solidity tuple
the counterparty's ICS26Router expects. Needed to submit a cbdc-node packet to
Besu, which is the return leg.
Return leg proven on the bench rig, cbdc-node -> Besu(Brazil), with real
contracts and real attestation:
1. 300 tCeBM_BRL escrowed on Besu, relayed in, voucher minted on cbdc-node.
2. Voucher sent back over qbftclient-1 and burned.
3. cosmos/ibc-attestor signed a StateAttestation; updateClient accepted it and
stored the consensus timestamp.
4. The same attestor signed a PacketAttestation over the commitment.
5. ICS26Router.recvPacket accepted it: escrow 800 -> 500, and the recipient
received the 300.
So the full round trip now holds: escrow in, voucher out, voucher back, escrow
released. That is the first of DEC-3's four templatisation requirements.
Two findings recorded in the deployment parameters:
The encoding mitigation is not actionable through the CLI. MsgTransfer carries an
Encoding field and MarshalPacketData supports EncodingABI, but cbdcd tx
ibc-transfer transfer has no --encoding flag, so it always sends the default that
ibc-go resolves to application/json -- which the Solidity side cannot decode. The
documented instruction to "set it explicitly" has no path through shipped tooling;
completing the leg needed --generate-only plus a jq injection. It belongs in the
relayer/issuer path, and a CLI flag is a one-line upstream contribution.
ibc-attestor's PacketAttestation expects ABI-encoded Solidity packets, not the
protobuf form cbdc-node emits. The protobuf form fails with "AbiError: type check
failed for offset (usize)", which names neither the expected format nor the field.
Also observed: the commitment stays open after the counterparty receives, because
it clears on acknowledgement -- so the money completes before the lifecycle does.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…s-only lists local-node.sh rewrote min_deposit's denom to axrp and left expedited_min_deposit at the SDK default of 50000000 "stake" -- a denom this chain does not have. So an expedited proposal could never meet its deposit and the fast governance path was silently unusable. It was accepted rather than rejected because Params.ValidateBasic compares the two with IsAllLTE, which does not trip across disjoint denom sets. The config is valid and useless: it passes every check while making the fast path impossible to fund. A genesis that failed validation would have said so at boot. That matters because the fast path is what you reach for given DEC-14's accepted deadlock, DEC-16's fast stop being unbuilt, and DEC-19 launching without quotas. Set to 2 axrp -- not 1, since validation requires it to be strictly greater than min_deposit, so 2 is the smallest value consistent with wanting no restriction. min_deposit itself stays 1 axrp and is confirmed deliberate: zero is not expressible, because sdk.Coins normalises zero amounts away and Empty() is rejected. Also recorded in DEC-14: the rate-limiting module's denom blacklist and sender/receiver whitelist are genesis-only with no messages and nothing wiring them, so both must stay empty. The whitelist is the dangerous half -- it bypasses rate limiting entirely without recording the flow, so it fails open, permanently and invisibly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
.gitignore covered only `.cbdcd/`, so `.cbdcd-pilot/` and `.cbdcd.bak-20260730-084348/` were untracked-but-offered. Both are node homes and both contain `keyring-test`, which must never reach the repository. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…e node The instrument behind the spike's Stage 1 result: it assembles an SP1 update-client fixture from a running cbdc-node, which is what let the stock `SP1ICS07Tendermint` program be run against real headers rather than synthetic ones. It was untracked while the docs already listed it under "built and proven" (docs/README.md), so the evidence for DEC-24 existed on one disk only. Verified: `go build ./cmd/sp1fixture/` clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ng it The rig ran `--bonsai-historical-block-limit=10000` / `--bonsai-trie-logs-pruning-window-size=20000` — about 5.5 hours at 2 s blocks, where DEC-21 decided 24 hours (43,200 blocks) for the Besu side. Retention is what bounds how far a relayer may lag: past the limit `eth_getProof` fails with "World state unavailable" and no proof can be produced for that height at all. Re-ran the rig to check the flags rather than assuming they parse: boots, produces blocks, no errors. Note Besu logs "Forcing --bonsai-limit-trie-logs-enabled=false, since it cannot be enabled with --sync-mode=FULL", so locally the pruning-window half is inert and the historical-block-limit is the flag doing the work. On LNET's spoke it will not be inert, so that ask is unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CI enforced 4 of the 13 packages that have tests. `make test-poa` resolves to ./x/poa/... only, so the Dockerfile's integration stage — which is what pull-request.yml builds — never ran the unit tests for x/qbftclient, x/qbftclient/prover, x/cbdc or app/ibc/corridorpause. `make test` had the same hole. That is the verification core DEC-28 cites as what replaced the external audit, the module 0.4b's pauser is about to land in, and the six test cases DEC-16 argued made the pauser cheap. Adds a test-unit target over the existing EXCLUDED_UNIT_PACKAGES, puts it in `make test` in place of test-poa (a subset of it), and calls it from the Dockerfile. CI now runs 15 packages. Also anchors that variable's `grep -v app`, which was dropping app/ibc/corridorpause from unit coverage as well as the app package it meant to exclude. ./app stays out: its only test is TestFullAppSimulation, which needs the -Enabled/-NumBlocks/-Params flags the test-sim-* targets pass and panics without them. Verified: `make test-unit` green across all 15 packages. Requirement 1 of DEC-29's assurance bar. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…pi groundwork Three components, built to close the gap between "the corridor works when I type" and "the corridor runs". x/qbftclient/attestor is the signing core for the outbound leg. Its encodings are pinned to the deployed contract rather than to themselves: the digest is sha256(tag || sha256(abi.encode(struct))) with the tag over the INNER hash, signed as a raw digest -- not EIP-191, since the contract calls ECDSA.recover directly -- and v shifted into OpenZeppelin's 27/28 range. Golden values come from cast, which was verified against the live client. Upstream's IBC_ATTESTOR_DESIGN.md is stale on both points and the package doc says so. cmd/qbftattestor is the attestor sidecar, and it exists as a separate process for one reason: an attestor never signs what it is told. It takes a height and a set of paths, reads the timestamp and every commitment from its own view of cbdc-node, and signs only that. Asking it to attest a packet the chain cannot see is refused. It also refuses a second, different timestamp for a height it has already attested, which is the only path to a terminal client freeze. It serves both an HTTP API and upstream's AggregatorService gRPC, so cosmos/ibc-relayer's cosmos-to-eth can consume it directly and ibc-attestor need not replace it. cmd/corridord drives both legs automatically. It holds no attestor key -- it asks the sidecar -- and no cbdc-node key: the inbound leg emits an unsigned tx and lets the chain's own tooling sign it, which is the boundary DEC-7 draws. It is a rig tool and says so; DEC-18 names cosmos/ibc-relayer as the driver, and this deviates deliberately because that path needs three components that do not exist yet. Also here: generated Go types for upstream's aggregator and proof-api services, with hand-written gRPC glue for the aggregator because the proto-builder image ships protoc-gen-go but not protoc-gen-go-grpc. local-node.sh gains SKIP_START so genesis can be seeded between init and start. That window is where denom metadata must be written -- ibc-go only synthesises voucher metadata if none exists and x/bank has no MsgSetDenomMetadata, so after start there is no second chance. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The shim (cmd/qbftproofapi) is the gate on using cosmos/ibc-relayer, because its proof-API endpoint is a single global config field rather than a per-route map, so one service must answer for both directions. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ICS26Router.getCommitment takes the keccak256 of the receipt path, not the path itself. Passing the raw bytes returned zero for every packet, so the already-delivered check always answered no and the daemon redelivered on every tick. Nothing bad happened only because IBC's own replay protection rejected the duplicates, and relying on that is not a design. The error path was wrong in the more dangerous direction too: an unreachable node was read as not-yet-delivered, which is the same redelivery loop triggered by a network blip. It now fails closed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The receipt check's "fail closed" comment was aspirational. Both call sites did `if recvd, _ :=`, discarding the error, so an RPC hiccup produced the zero value false -- read as "not delivered" -- and the redelivery loop the fix was supposed to remove. Now the error propagates and the tick aborts. Worse, the attestor's package doc claimed the freeze guard survives a restart because it "re-derives from the chain, which is deterministic". No such code exists. `seen` is in memory and starts empty, so the guard protects nothing in the one case that matters: local-node.sh does `rm -rf $HOMEDIR`, so re-genesis is the routine workflow, heights repeat with new timestamps, and a restarted sidecar will sign the new one against a light client still holding the old -- freezing it permanently and stranding the escrow. The comment now says what the code does, names CometBFT's per-height determinism as what actually protects the rig today, and records that the real fix is to bind attestations to a chain identity rather than to persist the map. Found by a defensive review of the running rig. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… log scan F9: the unsigned tx was written to a predictable /tmp path and then handed to `cbdcd tx sign --from alice`. A key gets applied to whatever sits at that path, so a local user could pre-create a symlink or swap the file between write and sign. Now a per-invocation 0700 dir via os.MkdirTemp, removed on return. F10: the SendPacket filter matched only the router address and topic0, then read Topics[2] as the sequence without checking Topics[1]. A string indexed parameter is stored as the keccak256 of the string, so logs from any other client on the same router were relayed as if they were ours. Now matched against keccak256(clientID), case-insensitively since eth_getLogs returns lowercase hex. The same query also rescanned from block 0 on every tick. It now starts from a cursor that advances only to blocks whose logs were actually returned -- never to the chain head, which could name a block the query did not cover and silently skip packets. The last log-bearing block is re-scanned inclusively, which is harmless because the receipt check drops duplicates, and an empty result leaves the cursor untouched. Residual: a reorg deeper than the last log-bearing block would not be re-fetched. QBFT finalises immediately so this cannot arise here; a reorg-capable chain would need a confirmation-depth lag. The cursor is in-memory, so a restart rescans from 0 -- consistent with this daemon's stated no-crash-resume scope. Fixes found by a defensive review of the running rig. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Four items from the defensive review. The `seen` map grew by one entry per attested height forever. Bounding it is not free, though, because that map IS the freeze guard: an evicted height is no longer guarded, so a later conflicting attestation for it would be signed and would freeze the client permanently. The eviction is therefore capped at 100k entries, logs loudly when it trims, and records in a comment that this is only acceptable because the guard is already insufficient across restarts, and that the real fix -- binding attestations to a chain identity -- removes the need for the map rather than merely bounding it. corridord read attestor error bodies with a single Read into a 512-byte buffer, so a refusal explaining why the attestor declined could be truncated to nothing useful. Now io.ReadAll behind an 8 KiB limit, so the message survives without letting a misbehaving endpoint stream forever. Also removed: an unused context import and its blank-assignment placeholder in the attestor, and three ABI type variables in the signing package that were declared and never used. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
F6: sendEvents asked for per_page=100 and never set page or read
total_count, so once cbdc-node holds more than 100 matching txs the
enumeration truncated silently -- packets past the cap would never be
relayed and the funds would sit escrowed with nothing logged. It now pages
through total_count, terminating on a short or empty page as well so a
server misreporting the total cannot spin it forever.
pendingSequences was also a lie: it returns every sequence ever sent and
filters nothing, and on this rig commitments never clear because acks are
not relayed. Renamed to allSentSequences with the reason recorded, so the
monotonic growth reads as a known scope limit rather than a filtering bug.
The relaying logic is unchanged -- the caller's receipt check is what
separates delivered from undelivered.
F3 cannot be fixed at this pin and the commit should say so plainly. The
signed payload is {height, timestamp} with no chain id, client id or
contract address, so a signature verifies against ANY light client trusting
that key. Widening the struct is not available: the verifier abi.decodes
the signed bytes into its own structs, so extra fields would break
verification for every deployed client, and forking the contract would
forfeit the only external audit left in the system. A test now pins the
encoding at exactly two words so a well-meaning "fix" trips a test rather
than the rig.
What is possible is process-level binding, so the attestor now requires
-cbdc-chain-id, -light-client and -besu-chain-id, and refuses to start when
the node's reported chain id does not match its configuration. Verified: it
exits rather than sign for a chain it was not configured for.
The residual is documented rather than papered over. This catches a wrong
RPC or a re-genesis under a NEW chain id; a re-genesis reusing the same id
still passes and still repeats heights with fresh timestamps, which is the
F2 freeze path. Domain separation needs an upstream struct change or a
different client.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
F2, the last high-severity defect. The guard against signing two different timestamps for one height -- which freezes the spoke's light client permanently and strands the escrow -- was an in-memory map that started empty on every restart. It protected nothing in the one scenario that matters, because local-node.sh does `rm -rf $HOMEDIR`, so re-genesis is the routine workflow: heights restart from 1 with new timestamps while the deployed client still holds the old ones. Two process-side mechanisms, because the payload cannot carry a chain identity: the contract abi.decodes the signed bytes into its own struct, so widening them breaks verification for every deployed client. The record is now an append-only seen.jsonl, fsynced BEFORE the signature is returned -- never after, or a crash between signing and writing reopens the hole -- and replayed at startup. A torn trailing record means a crash mid-append, so nothing was ever signed, and is truncated; a newline- terminated conflict is fatal. The block-1 hash is pinned on first run. It changes when a chain is re-genesised even under the same chain id, which is exactly what the existing -cbdc-chain-id check could not catch. On mismatch the process refuses to start and says why. -reset-state is the escape hatch for a genuine redeploy and logs loudly before wiping. Eviction now refuses rather than un-guards: below the low-water mark the guard declines to sign instead of silently losing the guarantee. The log itself is never trimmed -- 30 bytes per height is cheap against a permanently frozen corridor. Verified live: with the recorded block-1 hash altered to simulate a re-genesis, the sidecar refuses to start and names the consequence. Residual, none of it closed by this change: -reset-state is loud but unverified, since this process cannot confirm the light client was actually redeployed; deleting the state dir is indistinguishable from a first run; cross-replay remains possible because the payload has no domain separation; this guards one key, so unguarded co-signers at m-of-n freeze it anyway; and a Byzantine RPC serving two timestamps for one height wins on its first answer. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
F1. A failed cbdc-node -> Besu transfer could never return its funds, for
two independent reasons.
Timeouts were cryptographically impossible: ICS26Router.timeoutPacket needs
verifyNonMembership, which the attestation client accepts only for an
attested {keccak256(receiptPath), bytes32(0)} pair, and the sidecar had no
way to produce one -- an absent commitment was treated as an error and
refused. And acknowledgements were never relayed at all, so send
commitments never cleared.
The absence path is the most dangerous code here: signing "this packet was
never received" about one that WAS received releases escrow that must not
be released. It proves absence from the sidecar's own view and refuses
anything ambiguous. The trap that makes this non-trivial, found while
implementing it: the SDK's IAVL store answers a query for a pruned or
nonexistent version with code 0 and an empty value -- byte-identical to
genuine absence, differing only in Log. The countermeasure is prove=1,
which makes rootmulti hard-error on a missing version instead of returning
empty success, and acceptance now requires all of code 0, the response
height echoing the request, proof ops present, and an empty value. A
floating height is refused before the node is consulted.
Absence is deliberately NOT exposed over gRPC: upstream's request message
cannot express non-membership intent, and inferring it from an empty read
is precisely the failure mode the HTTP path was hardened against.
Ack relaying now runs in both directions -- Besu acks to cbdc-node via
qbftrelay's new -as-ack, cbdc acks to Besu via ICS26Router.ackPacket, each
gated on the counterparty commitment still being set. corridord keeps its
holds-no-keys property: attestation via the sidecar, signing via cbdcd.
Timeout submission stays operator-driven rather than automated, because the
daemon cannot evaluate the timeout safety condition without parsing packet
bodies out of Besu logs. The contract enforces it either way.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
It was written into a temporary checkout of solidity-ibc-eureka and would have been lost. It belongs here: it is our own contract, not a fork of upstream, so it does not touch the audit that forking would forfeit. Added contracts/spoke/ with a README recording why the contract exists at all -- ICS-20 carries only the denom string, IBCERC20 lost its setMetadata in v3.0.0, so a voucher's name can only be chosen by pre-registering a token before the first packet -- and the two warnings that go with it: the window shuts at the first packet, and setCustomERC20 validates nothing about the address it is given while that contract holds mint authority. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three real defects, all found by running things rather than reading them. x/cbdc's mint and burn were unusable from the CLI. autocli's Coin flag binder produces a pulsar Coin, x/cbdc has no pulsar codegen so its messages resolve to dynamicpb, and proto.Merge across the two descriptor families panics unconditionally inside BuildMsgMethodCommand -- no autocli option avoids it, because the merge happens before any of them apply. Hand-written cobra commands keep the same --address/--amount surface. Verified against the live chain: mint and burn both return code 0 and move the balance. Trade-off: tx cbdc update-params is no longer auto-generated, and it is gov-gated so it was not usable from the CLI anyway. corridord scanned eth_getLogs from block 0 to latest, which exceeds Besu's ~5000-block RPC range limit, so a fresh start against any non-trivial chain failed every tick with "Requested range exceeds maximum RPC range limit". It now scans in 4000-block windows and advances its cursor only over fully processed coverage. On restart it immediately cleared acks that had been stranded by this. The absence attestation added for F1 was dead on arrival: it sent prove=1, but CometBFT decodes prove as a JSON bool and 500s on 1, so every request was refused and the Besu refund path could never have worked. The unit test asserted "1" against a mock, which is why it passed while the real thing could not function -- both are now "true". A mock that agrees with the code rather than the server proves nothing. Also: outbound and inbound never checked whether the send commitment still existed, so a timed-out and refunded packet was retried forever. Both timeout directions are now exercised end to end. besu->cbdc via /attest/absence and ICS26Router.timeoutPacket, with the faucet's balance returning exactly. cbdc->besu via qbftrelay -as-timeout and MsgTimeout, with the escrowed amount returning exactly. Recorded rather than fixed: ahnl can never exist -- x/cbdc pins the denom to acbdc in the keeper -- so that pre-registration is unreachable. The named-token mechanism is proven instead on a denom the chain will carry. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… corridor qbftproofapi exists as a proof-API shim so this relayer can drive the corridor, so the relayer — not a hand-rolled watcher — is the intended component. This is the configuration it needs, plus what was learned trying to run it. The relayer starts clean against this config and its DELIVERY leg works: given a tx hash it built the proof through qbftproofapi and delivered on Besu. Two things stop it being usable as-is, both recorded here rather than in a chat log: - It has NO event loop. Nothing was relayed until Relay(tx_hash, chain_id) was called on its gRPC API; a send alone sits untouched. Whatever runs it must feed it hashes. - Its Cosmos signer derives a cosmos-style bech32 from the key, while cbdc-node is an Ethermint chain deriving eth_secp256k1 addresses. The ack leg fails as a result. Whether the relayer can sign for an Ethermint chain at all is a question for its owners; nothing here can work around it. A SEPARATE database, deliberately. The relayer dedupes on (client, sequence), and the shared one still holds rows from the retired devnet — including qbftclient-0 rows whose sequences a re-genesised chain restarts through, so new packets would collide with them and be silently never relayed. relayer-keys.json is gitignored and only the template is committed. The template carries the rule that cost real debugging twice: the relayer needs its OWN account per chain per leg. Sharing one with the sender, or between two legs, causes sequence contention that drops acknowledgements while the packet has already been delivered. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…we made Standing up a corridor leg means getting four things right that fail silently or irreversibly. Every one of them was got wrong at least once this week, so this script refuses to proceed rather than producing a corridor that looks fine and is not. Guard 1 — attestor key uniqueness. The signed attestation payload carries no domain separation: no chain id, no client id, no verifying contract (x/qbftclient/attestor/attestation.go:23-40). A signature is valid against ANY AttestationLightClient holding the key, so reuse across corridors makes attestations cross-replay — and since two chains disagree on the timestamp at a given height, a cross-replayed state attestation is a PERMANENT freeze. The attestor's own guard cannot help: it is a durable log under -state-dir, so two processes sharing a key share no guard at all. Verified against a rig that was already violating this, with the same key on both legs. Guard 2 — one relayer account per leg. Two processes signing from one account collide on the sequence number and the loser is dropped silently. Seen twice: sender vs relayer, then leg vs leg. Both times a packet was DELIVERED and its ack lost, leaving a commitment open behind money that had already moved — which is exactly the state an escrow-only monitor reports as settled. Guard 3 — signer derivation. cbdc-node accepts plain secp256k1 alongside its own eth_secp256k1 (devnet-findings §3.6), so cosmos/ibc-relayer needs no port. But qbftproofapi stamps -signer into Msg.Signer and the SDK requires msg-signer == tx-signer; hand it an eth-derived address while the relayer signs cosmos-derived and every ack fails, looking exactly like a curve incompatibility. It is not — that misreading cost most of a day. Guard 4 — process identity, not liveness. The first version grepped the log for "listening on", which the attestor prints BEFORE the bind can fail; the second probed the port, which succeeds against whichever leg already owns it. Both pass while the new leg relays through another leg's attestor, keyed for a different light client. It now compares the address the port reports against the address this leg's key derives, and refuses on mismatch. Caught by testing the guard rather than trusting it. The counterparty prefix asymmetry is applied rather than documented: a single empty element on Besu, two elements on Cosmos, both irreversible. Contract deployment stays out of scope — that is DeployCorridorHub.s.sol against a writable eureka checkout, whose build recipe is in corridor-deploy-inputs.md. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Self-review of the guards added yesterday. Two of the three were right only by accident, which for code whose whole job is refusing bad input is the same as being wrong. The attestor-key guard tested the wrong exit status. `grep ... | head` reports HEAD's status, and head exits 0 on empty input, so the branch was taken even when grep matched nothing — on a fresh registry prior_leg is empty, the equality fails and the guard dies with "already bound to leg ''". It would have blocked every FIRST deployment. It happened to work only because `pipefail` is set forty lines earlier; verified by running the same construct with pipefail off, where it misfires immediately. Now decided by awk on the field itself, so no shell option is load-bearing. Both registry lookups also used `grep -P`. PCRE is a GNU extension and absent on macOS and busybox, so the guards would have degraded into silence off Linux — worse than not having them, because the operator would believe they ran. Now awk, which also removes the tab-in-a-pattern quoting. The watcher's log parser did arithmetic on unvalidated fields: one malformed entry and `$((16#))` aborted the loop, taking every other packet in that batch with it. Skips the entry instead. Verified by making each guard fire and, for the first, by making it NOT fire on a fresh registry with pipefail explicitly disabled. Nothing here changes what the guards are for; it changes whether they work when the surrounding conditions are not exactly the ones I happened to test under. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…session narrative from comments Structural review of my own commits, on placement and comments rather than behaviour. Nothing here changes what any of it does. DeployCorridorHub.s.sol was in contracts/spoke/, whose README defines that directory as "Solidity we author for the Besu spoke" — contracts that are deployed as part of the system. A Foundry deploy script is not one: it is never deployed, and it cannot even build there, since that directory has no Foundry project and the script only compiles inside a writable eureka checkout. Anyone opening contracts/spoke/ would reasonably read it as part of the contract set. Moved to scripts/corridor/ alongside the tooling that uses it, with the two references updated. Comments in both scripts narrated how the traps were found — "hit at least once getting here", "observed twice: once between the sender and the relayer, once between two legs". The rule belongs in the source; the history belongs in git, where it already is. Rewritten to state the failure and its consequence without the first person. The comments stay long: this repo explains why, and these particular whys are the difference between a working corridor and a bricked one. Test files renamed to the convention the package already uses — ics20_abi_test.go names the file under test plus the aspect, so denom_route_test.go becomes ics02_denom_route_test.go and nonce_serialisation_test.go becomes ics20_nonce_test.go. Neither was findable from the file it tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…DK lacks ibc-go synthesises voucher metadata from the denom string alone and only writes it when none exists yet, so a voucher that arrives before its metadata is seeded is stuck at 0 decimals forever. This is the post-genesis correction path, implementing DEC-22. Gated on the gov authority rather than the mint/burn owner: how a currency presents itself is a monetary-presentation decision that should take a proposal, not an operational key. ValidateMetadataDisplayResolvable exists because x/bank's own Validate is not sufficient here. The erc20 precompile resolves decimals by matching Display against the denom units, but for an ibc/ base it matches only the LAST '/'-separated segment. x/bank requires a unit named after the FULL Display, so trace-qualified metadata passes SDK validation and still makes every decimals() call revert -- exactly the mislabelled voucher this message exists to fix, so it must not be able to write one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…elay through them The ack and timeout proofs were built inline in cmd/qbftrelay, which meant cmd/qbftproofapi had no shared path to reuse and the two could drift on the one detail that is easy to get wrong: receipt and ack are both keyed by the packet's DESTINATION client, because that is the store they were written into -- the mirror of RecvMsgs' source-client keying. Both now live in relaytx alongside RecvMsgs, and qbftrelay calls them. That also collapses the shadowed-err handling the inline version needed. TimeoutMsgs documents what makes an absence proof safe: it only says "not received AS OF target", and ibc-go additionally requires target's consensus timestamp to be past the packet's timeout, after which the receipt can never legally appear. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The shim translated recv only. Acks and timeouts travel AGAINST their packet, so each proof direction has to serve packets sent both ways before the relayer can close a round trip on its own. Proofs FROM Besu reuse relaytx's Ack/TimeoutMsgs, so this and cmd/qbftrelay cannot drift on the destination-client keying. Attestations FROM cbdc-node need a second sidecar address: receipt ABSENCE is deliberately not on the AggregatorService surface, because upstream's GetAttestationsRequest cannot distinguish "attest this value" from "attest there is no value". Timeouts therefore go through the sidecar's HTTP /attest/absence, the only surface where that intent is explicit. The sidecar still verifies against its own cbdc-node view before signing, so a caller cannot smuggle in an absence any more than a commitment. Holds no keys of any kind, unchanged (DEC-7). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
BaseDenom is not just the EVM default; app.go passes it to the x/cbdc keeper, which rejects every other denom with ErrInvalidDenom. With acbdc here, every mint failed inside x/group -- which reports the proposal as ACCEPTED while the inner message failed, so issuance silently did nothing. Recorded in the comment because the failure mode gives no signal at the proposal layer, and because the denom is NOT persisted (x/cbdc Params carry only owner, issuance_paused and paused_ibc_clients), so changing it is a binary change rather than a migration. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…lit display from symbol Two mistakes this script made easy, both uncorrectable after the first packet because ibc-go only synthesises metadata when none exists. <client-id> must be the client on THIS chain tracking the source, not the one the sender passes to `tx ibc-transfer transfer`. ICS-20 prefixes the denom with the destination-side client. Verified on a live v2 transfer: a send over 07-tendermint-1 landed as transfer/07-tendermint-0/<denom>. display is now a separate optional argument defaulting to symbol. x/bank requires display to name one of the denom_units, but a '/' -- legal in a denom, awkward as an ERC20 symbol() -- is exactly why the two need to be separable. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… per-run state autorelay.sh drives both directions off persisted height cursors under STATE_ROOT; relayer-config.scenb.yml points cosmos/ibc-relayer at the Scenario B hub leg. Both write their state under $PWD/.corridor, which is the repo root whenever the corridor is driven from here, so it is ignored: logs, the attestor seen-set and relay height cursors are per-run, not source. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.