feat(attester): IBC-compatible attester overlay with signed votes#392
Open
feat(attester): IBC-compatible attester overlay with signed votes#392
Conversation
…idation - Add consensus_address field (field 4) to AttesterInfo proto and regenerate pb.go - Add types/attester.go with NewAttesterInfo, GetPubKey, and UnpackInterfaces helpers - Rewrite InitGenesis to materialise AttesterInfo, AttesterSet, ValidatorIndex, and ValidatorPower from genesis attester_infos, sorted by pubkey address ascending to match CometBFT NewValidatorSet ordering; enforce pubkey/address match - Rewrite ExportGenesis to round-trip attester_infos; ValidatorIndices no longer exported - Add keeper/genesis_test.go covering load, mismatch rejection, and export round-trip
Wire verifyVote into the Attest handler so every attestation is cryptographically verified. Remove the SIGN_MODE_CHECKPOINT gate (every height is now attestable) and the MinVoteLen constant (superseded by the full proto decode + signature check inside verifyVote). Update TestAttestHeightBounds to seed attesters directly via keeper calls and build properly signed votes via signTestVote. Update TestAttest and TestAttestVotePayloadValidation similarly.
Both handlers now return ErrInvalidRequest with "attester set changes disabled" since the set is fixed at genesis. Remove MaxAttesters const which was only enforced at join time.
…d quorum gate Add AttesterSet RPC to the network query service so getCommitForHeight can build a cmttypes.Commit that 07-tendermint accepts: one entry per validator in canonical index order, BlockIDFlagAbsent for non-signers, and a hard refusal to serve until more than 2/3 of the set has signed.
…ommitLight Add AddGenesisAttester post-init hook that writes one AttesterInfo entry to app_state.network.attester_infos using the node's ed25519 consensus pubkey and the operator address from the validator keyring entry. Wire the hook into getGmChain so the attester set is populated before genesis is finalized. Without this, MsgAttest rejects votes from unregistered keys and LastAttestedHeight stays 0, breaking Hermes IBC client creation. After the attester starts, wait for the chain to reach height 10 and assert that VerifyCommitLight passes on the fetched commit as an early failure signal before Hermes setup.
…l (prefix-agnostic)
Attesters previously could sign a well-formed cmtproto.Vote over an arbitrary BlockID.Hash — the signature self-verified and the bitmap bit was set, but the reconstructed commit failed 07-tendermint VerifyCommitLight on IBC counterparties. verifyVote now fetches the canonical BlockID from the adapter store via a new BlockIDProvider interface, rejecting mismatches. Provider is wired post-depinject via an app-side networkKeeperBlockIDWirer; if unwired, verification fails closed. Includes regression tests for mismatched and unwired-provider paths.
Contributor
📝 WalkthroughWalkthroughGenesis now loads an immutable, deterministically-indexed attester set; attestation handling verifies CometBFT precommit votes against a wired BlockID provider, disables join/leave, and exposes the attester set via gRPC. RPC commit reconstruction and consensus RPCs derive data from the fixed attester set. Changes
Sequence Diagram(s)sequenceDiagram
participant Attester
participant Keeper as NetworkKeeper
participant BlockIDProvider
participant VoteVerifier
participant Quorum
Attester->>Keeper: MsgAttest(consensusAddr, voteBytes)
Keeper->>VoteVerifier: verifyVote(consensusAddr, voteBytes, msgHeight)
VoteVerifier->>VoteVerifier: Unmarshal & validate Precommit (type,height,round)
VoteVerifier->>VoteVerifier: Ensure ValidatorAddress matches pubkey
VoteVerifier->>VoteVerifier: Verify signature against VoteSignBytes
VoteVerifier->>BlockIDProvider: GetBlockID(height)
BlockIDProvider-->>VoteVerifier: BlockID
VoteVerifier->>VoteVerifier: Compare vote.BlockID == stored BlockID
VoteVerifier-->>Keeper: Verified vote
Keeper->>Keeper: Persist signature & bitmap
Keeper->>Quorum: Evaluate votedPower*3 > totalPower*2
Quorum-->>Keeper: quorum met / not met
Keeper-->>Attester: response (accepted/rejected)
sequenceDiagram
participant RPC
participant ABCI
participant Keeper as NetworkKeeper
participant BlockStore
participant ValidatorSet
RPC->>ABCI: Query AttesterSet
ABCI->>Keeper: iterate ValidatorIndex -> AttesterInfo
Keeper-->>ABCI: ordered AttesterSet
RPC->>ABCI: Query AttesterSignatures(height)
ABCI->>Keeper: return stored signatures map
RPC->>RPC: assemble cmttypes.Commit (ordered entries, absent flags)
RPC->>RPC: require signedCount*3 > total*2
RPC->>BlockStore: get validator set for height
BlockStore-->>RPC: ValidatorSet
RPC->>ValidatorSet: VerifyCommitLight(commit)
ValidatorSet-->>RPC: result
RPC-->>caller: commit / error
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Remove unused perHeightBlockIDProvider helper and extra blank line flagged by gci/unused.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (8)
pkg/adapter/providers_test.go (1)
60-62: Use a non-nilfallback pubkey to reduce test brittleness.Line 61 currently passes
nilaspubKey; it works only because this case does not hit the fallback branch. Passing a real key makes the test more future-proof.Suggested change
- gotHash, err := hasher(sequencerAddr, nil) + gotHash, err := hasher(sequencerAddr, libp2pCanonical[0])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/adapter/providers_test.go` around lines 60 - 62, The test calls ValidatorHasherFromStoreProvider(store) to get hasher and then invokes hasher(sequencerAddr, nil); replace the nil pubKey with a real non-nil fallback public key to avoid relying on current branch behavior — locate the hasher invocation (variable hasher from ValidatorHasherFromStoreProvider) and pass a valid pubKey value (e.g., a test key or small byte slice representing a public key) instead of nil so the test remains robust if the fallback branch is exercised in future.pkg/rpc/core/consensus_test.go (1)
325-328: Test may be fragile due to assumed ordering.The assertion at line 326 assumes
result.Validatorsare in the same order asprivs. However, CometBFT'sNewValidatorSetsorts validators by address, andbuildValidatorSetFromAttesterSetiterates over entries in index order. If the setEntries are constructed in a different order than the expected sorted order, this test could fail intermittently.Consider either:
- Sorting
privsby their pubkey address before creating setEntries, or- Using a map-based lookup for validation instead of index-based comparison.
💡 Suggested fix
+ // Sort privs by address to match expected validator set ordering + sort.Slice(privs, func(i, j int) bool { + return bytes.Compare(privs[i].PubKey().Address(), privs[j].PubKey().Address()) < 0 + }) + setEntries := make([]networktypes.AttesterSetEntry, 0, len(privs))You'll need to add
"bytes"to the imports.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/rpc/core/consensus_test.go` around lines 325 - 328, The test assumes result.Validators are ordered like privs which is fragile because NewValidatorSet sorts by address and buildValidatorSetFromAttesterSet uses index order; fix by making the comparison order-independent: either sort privs by pubkey address before building setEntries (compare privs sorted by bytes.Compare(privs[i].PubKey().Address(), ...)) or replace the index-based assertions with a map lookup that checks each validator.Address exists in a map from privs' addresses and has VotingPower==1; also add the "bytes" import if you choose the sorting approach.modules/network/types/attester.go (1)
36-40: Add nil check in UnpackInterfaces to prevent potential panic.If
a.Pubkeyis nil (e.g., for a malformed or incompleteAttesterInfo), callingunpacker.UnpackAny(a.Pubkey, &pk)may behave unexpectedly depending on the unpacker implementation. A defensive nil check would make this more robust.🛡️ Suggested fix
// UnpackInterfaces ensures GetCachedValue works after unmarshaling. func (a AttesterInfo) UnpackInterfaces(unpacker codectypes.AnyUnpacker) error { + if a.Pubkey == nil { + return nil + } var pk cryptotypes.PubKey return unpacker.UnpackAny(a.Pubkey, &pk) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/network/types/attester.go` around lines 36 - 40, The UnpackInterfaces method (AttesterInfo.UnpackInterfaces) should defensively check a.Pubkey for nil before calling unpacker.UnpackAny to avoid a potential panic; update the method to return early (nil error) if a.Pubkey == nil, otherwise call unpacker.UnpackAny(a.Pubkey, &pk) as before, referencing the AttesterInfo type, its Pubkey field, the cryptotypes.PubKey variable pk, and the unpacker.UnpackAny call.pkg/rpc/core/consensus.go (1)
29-29: Consider reducing log level for per-request logging.Logging at
Infolevel for every/validatorsRPC call could generate excessive log volume under load. Consider usingDebuglevel or removing this log statement entirely.💡 Suggested change
- env.Logger.Info("Validators endpoint in attester mode - returning full attester set", "height", height) + env.Logger.Debug("Validators endpoint in attester mode - returning full attester set", "height", height)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/rpc/core/consensus.go` at line 29, Reduce per-request logging noise by changing the env.Logger.Info call that emits "Validators endpoint in attester mode - returning full attester set" to a lower level (use env.Logger.Debug) or remove it entirely in the RPC handler for the /validators endpoint; locate the log invocation (the env.Logger.Info(...) statement in consensus.go) and replace with env.Logger.Debug(...) so frequent RPC calls no longer produce Info-level logs, and run tests/local server to confirm behavior.server/attester_cmd.go (1)
194-200: Attestation errors are logged but not retried — verify this is intentional.The loop logs attestation failures and moves on without retry. For transient failures (network issues), this means the height won't be re-attempted until the next ticker iteration (if ever, since
nextHeightadvances). Consider whether failed heights should be queued for retry.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/attester_cmd.go` around lines 194 - 200, The loop advances nextHeight unconditionally after calling submitAttestation, so transient failures from submitAttestation are only logged and never retried; update the logic around the for loop and nextHeight to retry or queue failed heights: either (A) do not increment nextHeight past a failed height (i.e., break/return on error so the same h is retried on the next tick), or (B) push failed h into a retry queue/map with metadata (attempt count/timestamp) and process that queue alongside the main loop; modify the code referencing submitAttestation, nextHeight and the loop that iterates h to implement the chosen retry strategy and ensure retry bounds and backoff are applied to avoid tight loops.pkg/rpc/core/blocks.go (2)
401-436: Consider removing redundant sort — query endpoint already orders by index.The
AttesterSetgRPC endpoint atmodules/network/keeper/grpc_query.go:263already sorts entries by index before returning. The sort at line 421 is defensive but redundant.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/rpc/core/blocks.go` around lines 401 - 436, Remove the redundant sort by deleting the sort.Slice(resp.Entries, func(i, j int) bool { return resp.Entries[i].Index < resp.Entries[j].Index }) call in getAttesterSet and rely on the ordering guaranteed by the AttesterSet gRPC response (resp.Entries); optionally add a brief comment above the loop noting that entries are already ordered by index by the network module to make the assumption explicit.
470-476: Test helper uses global state swap pattern — adequate for single-threaded tests.The
GetCommitForHeightForTesthelper temporarily swaps the globalenvvariable. This is acceptable for test code but would be problematic if tests run in parallel. Ensure test files using this helper don't uset.Parallel().🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/rpc/core/blocks.go` around lines 470 - 476, GetCommitForHeightForTest temporarily swaps the package-global env which is not safe for parallel tests; either (A) mark the helper clearly as NOT parallel-safe in its doc comment and remove any t.Parallel() usage in tests that call GetCommitForHeightForTest, or (B) refactor the helper to avoid global state by adding a new function that accepts an Environment (e.g., GetCommitForHeightWithEnv(ctx, e *Environment, height uint64)) and have getCommitForHeight take env as an explicit parameter so callers can pass e without mutating the package-global env; update callers/tests to use the non-global variant if you choose the refactor.modules/network/keeper/attester_ibc_test.go (1)
291-301: Ascending address assertion may fail for absent validators.The sanity check at lines 291-301 only compares addresses of validators with
BlockIDFlagCommit. If an absent validator falls between two signing validators in address order,prevwon't update and the comparison could pass incorrectly. However, since the test signs all attesters in the main test, this is benign for current usage.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/network/keeper/attester_ibc_test.go` around lines 291 - 301, The ascending-address sanity check only updates prev when s.BlockIDFlag == cmttypes.BlockIDFlagCommit, which can skip addresses for absent validators and make the comparison invalid; instead, always update prev to s.ValidatorAddress on each iteration of the sigs loop while keeping the require.True check only for commit flags (i.e., use the existing sigs loop and BlockIDFlagCommit assertion but move the prev = s.ValidatorAddress assignment outside/after the commit-only branch so prev reflects the last seen validator address whether or not they committed).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@modules/network/genesis.go`:
- Line 65: Add a bounds check in InitGenesis to prevent casting overflow when
populating validator indices: before iterating over attesters and calling
k.SetValidatorIndex(ctx, info.ConsensusAddress, uint16(idx), 1) validate that
len(attesters) <= math.MaxUint16 and return an error (fmt.Errorf) if it exceeds
the limit; this prevents collisions in the ValidatorIndex/ValidatorPower maps
caused by silent uint16 overflow.
In `@modules/network/keeper/keeper.go`:
- Around line 256-264: CheckQuorum currently validates params.QuorumFraction but
ignores it (uses hardcoded 2/3); update CheckQuorum to actually use the parsed
quorum decimal from k.GetParams(ctx) (parsed via
math.LegacyNewDecFromStr(params.QuorumFraction)) to compute the required
threshold against totalPower instead of votedPower*3 > totalPower*2.
Specifically: parse params.QuorumFraction into a sdk.Dec (handle the parse error
as already done), convert totalPower and votedPower to Dec (e.g.,
sdk.NewDecFromInt or equivalent), compute required :=
quorumDec.Mul(totalPowerDec) and return whether votedPowerDec is greater-than
(or greater-or-equal if desired) required; keep the existing error handling path
for invalid fraction. Ensure this change in CheckQuorum aligns behavior with
SoftConfirmationStatus which returns the configured QuorumFraction.
In `@modules/network/types/genesis.go`:
- Around line 10-20: After deserializing the genesis with cdc.MustUnmarshalJSON
in AppModule.InitGenesis (modules/network/module.go), call the
GenesisState.UnpackInterfaces method to resolve Any-typed pubkeys; specifically,
after the existing cdc.MustUnmarshalJSON into the genesisState variable, invoke
genesisState.UnpackInterfaces(cdc) (or the appropriate AnyUnpacker) and
handle/return any error to ensure AttesterInfo.GetPubKey() will not fail;
reference GenesisState.UnpackInterfaces and InitGenesis when making the change.
In `@server/attester_cmd.go`:
- Around line 499-502: getLatestHeight currently returns 0 when
blockResp.Result.Block.Header.Height == "" which can silently stall the caller
loop (for h := nextHeight; h <= currentHeight). Change getLatestHeight to return
a sentinel negative value (e.g., -1, nil) when Height is empty and do not treat
that as a valid height; then update the caller that iterates using currentHeight
(the for h := nextHeight; h <= currentHeight loop) to check if currentHeight <=
0 and skip/poll without attempting to process ranges in that case. Reference
getLatestHeight and blockResp.Result.Block.Header.Height in your changes and
adjust the caller's loop guard to explicitly handle currentHeight <= 0.
---
Nitpick comments:
In `@modules/network/keeper/attester_ibc_test.go`:
- Around line 291-301: The ascending-address sanity check only updates prev when
s.BlockIDFlag == cmttypes.BlockIDFlagCommit, which can skip addresses for absent
validators and make the comparison invalid; instead, always update prev to
s.ValidatorAddress on each iteration of the sigs loop while keeping the
require.True check only for commit flags (i.e., use the existing sigs loop and
BlockIDFlagCommit assertion but move the prev = s.ValidatorAddress assignment
outside/after the commit-only branch so prev reflects the last seen validator
address whether or not they committed).
In `@modules/network/types/attester.go`:
- Around line 36-40: The UnpackInterfaces method (AttesterInfo.UnpackInterfaces)
should defensively check a.Pubkey for nil before calling unpacker.UnpackAny to
avoid a potential panic; update the method to return early (nil error) if
a.Pubkey == nil, otherwise call unpacker.UnpackAny(a.Pubkey, &pk) as before,
referencing the AttesterInfo type, its Pubkey field, the cryptotypes.PubKey
variable pk, and the unpacker.UnpackAny call.
In `@pkg/adapter/providers_test.go`:
- Around line 60-62: The test calls ValidatorHasherFromStoreProvider(store) to
get hasher and then invokes hasher(sequencerAddr, nil); replace the nil pubKey
with a real non-nil fallback public key to avoid relying on current branch
behavior — locate the hasher invocation (variable hasher from
ValidatorHasherFromStoreProvider) and pass a valid pubKey value (e.g., a test
key or small byte slice representing a public key) instead of nil so the test
remains robust if the fallback branch is exercised in future.
In `@pkg/rpc/core/blocks.go`:
- Around line 401-436: Remove the redundant sort by deleting the
sort.Slice(resp.Entries, func(i, j int) bool { return resp.Entries[i].Index <
resp.Entries[j].Index }) call in getAttesterSet and rely on the ordering
guaranteed by the AttesterSet gRPC response (resp.Entries); optionally add a
brief comment above the loop noting that entries are already ordered by index by
the network module to make the assumption explicit.
- Around line 470-476: GetCommitForHeightForTest temporarily swaps the
package-global env which is not safe for parallel tests; either (A) mark the
helper clearly as NOT parallel-safe in its doc comment and remove any
t.Parallel() usage in tests that call GetCommitForHeightForTest, or (B) refactor
the helper to avoid global state by adding a new function that accepts an
Environment (e.g., GetCommitForHeightWithEnv(ctx, e *Environment, height
uint64)) and have getCommitForHeight take env as an explicit parameter so
callers can pass e without mutating the package-global env; update callers/tests
to use the non-global variant if you choose the refactor.
In `@pkg/rpc/core/consensus_test.go`:
- Around line 325-328: The test assumes result.Validators are ordered like privs
which is fragile because NewValidatorSet sorts by address and
buildValidatorSetFromAttesterSet uses index order; fix by making the comparison
order-independent: either sort privs by pubkey address before building
setEntries (compare privs sorted by bytes.Compare(privs[i].PubKey().Address(),
...)) or replace the index-based assertions with a map lookup that checks each
validator.Address exists in a map from privs' addresses and has VotingPower==1;
also add the "bytes" import if you choose the sorting approach.
In `@pkg/rpc/core/consensus.go`:
- Line 29: Reduce per-request logging noise by changing the env.Logger.Info call
that emits "Validators endpoint in attester mode - returning full attester set"
to a lower level (use env.Logger.Debug) or remove it entirely in the RPC handler
for the /validators endpoint; locate the log invocation (the
env.Logger.Info(...) statement in consensus.go) and replace with
env.Logger.Debug(...) so frequent RPC calls no longer produce Info-level logs,
and run tests/local server to confirm behavior.
In `@server/attester_cmd.go`:
- Around line 194-200: The loop advances nextHeight unconditionally after
calling submitAttestation, so transient failures from submitAttestation are only
logged and never retried; update the logic around the for loop and nextHeight to
retry or queue failed heights: either (A) do not increment nextHeight past a
failed height (i.e., break/return on error so the same h is retried on the next
tick), or (B) push failed h into a retry queue/map with metadata (attempt
count/timestamp) and process that queue alongside the main loop; modify the code
referencing submitAttestation, nextHeight and the loop that iterates h to
implement the chosen retry strategy and ensure retry bounds and backoff are
applied to avoid tight loops.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7a84a8a2-2c39-457a-8c59-afb131b71e07
⛔ Files ignored due to path filters (4)
modules/network/types/attester.pb.gois excluded by!**/*.pb.gomodules/network/types/genesis.pb.gois excluded by!**/*.pb.gomodules/network/types/query.pb.gois excluded by!**/*.pb.gomodules/network/types/query.pb.gw.gois excluded by!**/*.pb.gw.go
📒 Files selected for processing (27)
.github/workflows/migration_test.ymlmodules/network/genesis.gomodules/network/keeper/abci.gomodules/network/keeper/attester_ibc_test.gomodules/network/keeper/bitmap_test.gomodules/network/keeper/genesis_test.gomodules/network/keeper/grpc_query.gomodules/network/keeper/keeper.gomodules/network/keeper/msg_server.gomodules/network/keeper/msg_server_test.gomodules/network/keeper/testhelpers_test.gomodules/network/types/attester.gomodules/network/types/expected_keepers.gomodules/network/types/genesis.gomodules/proto/evabci/network/v1/attester.protomodules/proto/evabci/network/v1/genesis.protomodules/proto/evabci/network/v1/query.protopkg/adapter/providers_test.gopkg/rpc/core/blocks.gopkg/rpc/core/commit_reconstruction_test.gopkg/rpc/core/consensus.gopkg/rpc/core/consensus_test.goserver/attester_cmd.goserver/start.gotests/integration/docker/Dockerfile.gmtests/integration/docker/patches/app-wiring/patch-app-wiring.shtests/integration/gm_gaia_health_test.go
| if err := k.SetAttesterSetMember(ctx, info.ConsensusAddress); err != nil { | ||
| return fmt.Errorf("set attester set member: %w", err) | ||
| } | ||
| if err := k.SetValidatorIndex(ctx, info.ConsensusAddress, uint16(idx), 1); err != nil { |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if there's a MaxAttesters constant or validation elsewhere
rg -n 'MaxAttesters|max.*attester|65535' modules/network/Repository: evstack/ev-abci
Length of output: 41
🏁 Script executed:
# First, let's read the genesis.go file to see the context
wc -l modules/network/genesis.goRepository: evstack/ev-abci
Length of output: 90
🏁 Script executed:
# Read the relevant section around line 65
sed -n '50,80p' modules/network/genesis.goRepository: evstack/ev-abci
Length of output: 1191
🏁 Script executed:
# Search the entire codebase for validator count limits, not just modules/network/
rg -n 'MaxValidators|MaxAttester|attester.*limit|validator.*count|uint16.*idx' --type goRepository: evstack/ev-abci
Length of output: 256
🏁 Script executed:
# Look for SetValidatorIndex method to understand what it does
rg -n 'SetValidatorIndex' --type go -A 5 -B 2Repository: evstack/ev-abci
Length of output: 5013
🏁 Script executed:
# Search for attester set size limits in proto files and constants
rg -n 'MaxAttester|max_attester|attester.*count|repeated.*Attester' --type protoRepository: evstack/ev-abci
Length of output: 89
🏁 Script executed:
# Check the genesis types and any validation
rg -n 'type.*Genesis|attester.*list|attester.*length' modules/network/types/ --type go -A 3Repository: evstack/ev-abci
Length of output: 435
🏁 Script executed:
# Look for any bounds checking or validation on attester count
rg -n 'len\(attesters\)|len\(.*attester|> 65535|attester.*validate' modules/network/ --type goRepository: evstack/ev-abci
Length of output: 539
🏁 Script executed:
# Look for NewBitmap implementation and any limits it enforces
rg -n 'NewBitmap|bitmapHelper' modules/network/ --type go -A 10 | head -100Repository: evstack/ev-abci
Length of output: 8190
🏁 Script executed:
# Search for any validation in InitGenesis or related functions
rg -n 'func InitGenesis|func.*InitGenesis|validate.*attester|validate.*genesis' modules/network/ --type go -A 10Repository: evstack/ev-abci
Length of output: 3361
🏁 Script executed:
# Check if there are any constants defining validator/attester limits
rg -n 'const.*\(|MaxValidator|MaxAttester|MAX_' modules/network/ --type go | head -50Repository: evstack/ev-abci
Length of output: 144
🏁 Script executed:
# Check the constants defined
rg -n 'const \(' modules/network/types/ --type go -A 30 | head -100Repository: evstack/ev-abci
Length of output: 4528
🏁 Script executed:
# Look at BitmapHelper implementation to understand size constraints
rg -n 'BitmapHelper|NewBitmap|SetBit' modules/network/keeper/ --type go -B 5 -A 15 | head -150Repository: evstack/ev-abci
Length of output: 11748
🏁 Script executed:
# Find BitmapHelper implementation
fd -n 'bitmap' modules/network/ --type fRepository: evstack/ev-abci
Length of output: 290
🏁 Script executed:
# Find the bitmap helper file
fd 'bitmap' modules/network/keeper/Repository: evstack/ev-abci
Length of output: 130
🏁 Script executed:
# Look at bitmap.go
cat modules/network/keeper/bitmap.goRepository: evstack/ev-abci
Length of output: 2088
Add validation to prevent attester set from exceeding uint16 max (65535).
The validator index is cast to uint16(idx) without bounds checking. If the attester set exceeds 65535 members, the cast silently overflows, causing multiple attesters to map to the same index and creating collisions in ValidatorIndex and ValidatorPower maps. Add a length validation in InitGenesis:
if len(attesters) > math.MaxUint16 {
return fmt.Errorf("attester set size %d exceeds maximum %d", len(attesters), math.MaxUint16)
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@modules/network/genesis.go` at line 65, Add a bounds check in InitGenesis to
prevent casting overflow when populating validator indices: before iterating
over attesters and calling k.SetValidatorIndex(ctx, info.ConsensusAddress,
uint16(idx), 1) validate that len(attesters) <= math.MaxUint16 and return an
error (fmt.Errorf) if it exceeds the limit; this prevents collisions in the
ValidatorIndex/ValidatorPower maps caused by silent uint16 overflow.
Comment on lines
256
to
264
| // CheckQuorum checks if the voted power meets quorum | ||
| func (k Keeper) CheckQuorum(ctx sdk.Context, votedPower, totalPower uint64) (bool, error) { | ||
| params := k.GetParams(ctx) | ||
| quorumFrac, err := math.LegacyNewDecFromStr(params.QuorumFraction) | ||
| if err != nil { | ||
| if _, err := math.LegacyNewDecFromStr(params.QuorumFraction); err != nil { | ||
| return false, fmt.Errorf("invalid quorum fraction: %w", err) | ||
| } | ||
|
|
||
| requiredPower := math.LegacyNewDec(int64(totalPower)).Mul(quorumFrac).TruncateInt().Uint64() | ||
| return votedPower >= requiredPower, nil | ||
| return votedPower*3 > totalPower*2, nil | ||
| } |
Contributor
There was a problem hiding this comment.
QuorumFraction parameter is validated but ignored — creates config/behavior mismatch.
The function validates that params.QuorumFraction is a valid decimal but then uses a hardcoded votedPower*3 > totalPower*2 check (fixed 2/3 threshold). This creates inconsistency:
- Operators can configure
QuorumFractionin params SoftConfirmationStatusquery returns the configured value- But the actual quorum check ignores it
Either remove the configurable parameter or use it in the check.
Suggested fix: Either use the parameter or remove validation
Option A: Use the parameter
func (k Keeper) CheckQuorum(ctx sdk.Context, votedPower, totalPower uint64) (bool, error) {
params := k.GetParams(ctx)
- if _, err := math.LegacyNewDecFromStr(params.QuorumFraction); err != nil {
+ quorum, err := math.LegacyNewDecFromStr(params.QuorumFraction)
+ if err != nil {
return false, fmt.Errorf("invalid quorum fraction: %w", err)
}
-
- return votedPower*3 > totalPower*2, nil
+ requiredPower := quorum.MulInt64(int64(totalPower)).TruncateInt64()
+ return int64(votedPower) > requiredPower, nil
}Option B: Remove the parameter validation (if 2/3 is intentionally fixed)
func (k Keeper) CheckQuorum(ctx sdk.Context, votedPower, totalPower uint64) (bool, error) {
- params := k.GetParams(ctx)
- if _, err := math.LegacyNewDecFromStr(params.QuorumFraction); err != nil {
- return false, fmt.Errorf("invalid quorum fraction: %w", err)
- }
-
+ // Fixed 2/3 quorum required for IBC light client compatibility
return votedPower*3 > totalPower*2, nil
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // CheckQuorum checks if the voted power meets quorum | |
| func (k Keeper) CheckQuorum(ctx sdk.Context, votedPower, totalPower uint64) (bool, error) { | |
| params := k.GetParams(ctx) | |
| quorumFrac, err := math.LegacyNewDecFromStr(params.QuorumFraction) | |
| if err != nil { | |
| if _, err := math.LegacyNewDecFromStr(params.QuorumFraction); err != nil { | |
| return false, fmt.Errorf("invalid quorum fraction: %w", err) | |
| } | |
| requiredPower := math.LegacyNewDec(int64(totalPower)).Mul(quorumFrac).TruncateInt().Uint64() | |
| return votedPower >= requiredPower, nil | |
| return votedPower*3 > totalPower*2, nil | |
| } | |
| // CheckQuorum checks if the voted power meets quorum | |
| func (k Keeper) CheckQuorum(ctx sdk.Context, votedPower, totalPower uint64) (bool, error) { | |
| params := k.GetParams(ctx) | |
| quorum, err := math.LegacyNewDecFromStr(params.QuorumFraction) | |
| if err != nil { | |
| return false, fmt.Errorf("invalid quorum fraction: %w", err) | |
| } | |
| requiredPower := quorum.MulInt64(int64(totalPower)).TruncateInt64() | |
| return int64(votedPower) > requiredPower, nil | |
| } |
Suggested change
| // CheckQuorum checks if the voted power meets quorum | |
| func (k Keeper) CheckQuorum(ctx sdk.Context, votedPower, totalPower uint64) (bool, error) { | |
| params := k.GetParams(ctx) | |
| quorumFrac, err := math.LegacyNewDecFromStr(params.QuorumFraction) | |
| if err != nil { | |
| if _, err := math.LegacyNewDecFromStr(params.QuorumFraction); err != nil { | |
| return false, fmt.Errorf("invalid quorum fraction: %w", err) | |
| } | |
| requiredPower := math.LegacyNewDec(int64(totalPower)).Mul(quorumFrac).TruncateInt().Uint64() | |
| return votedPower >= requiredPower, nil | |
| return votedPower*3 > totalPower*2, nil | |
| } | |
| // CheckQuorum checks if the voted power meets quorum | |
| func (k Keeper) CheckQuorum(ctx sdk.Context, votedPower, totalPower uint64) (bool, error) { | |
| // Fixed 2/3 quorum required for IBC light client compatibility | |
| return votedPower*3 > totalPower*2, nil | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@modules/network/keeper/keeper.go` around lines 256 - 264, CheckQuorum
currently validates params.QuorumFraction but ignores it (uses hardcoded 2/3);
update CheckQuorum to actually use the parsed quorum decimal from
k.GetParams(ctx) (parsed via math.LegacyNewDecFromStr(params.QuorumFraction)) to
compute the required threshold against totalPower instead of votedPower*3 >
totalPower*2. Specifically: parse params.QuorumFraction into a sdk.Dec (handle
the parse error as already done), convert totalPower and votedPower to Dec
(e.g., sdk.NewDecFromInt or equivalent), compute required :=
quorumDec.Mul(totalPowerDec) and return whether votedPowerDec is greater-than
(or greater-or-equal if desired) required; keep the existing error handling path
for invalid fraction. Ensure this change in CheckQuorum aligns behavior with
SoftConfirmationStatus which returns the configured QuorumFraction.
Comment on lines
+10
to
+20
| // UnpackInterfaces ensures AttesterInfo.Pubkey Any values have their cached | ||
| // concrete value populated after genesis JSON unmarshaling. Without this, the | ||
| // cosmos-sdk codec leaves the Any unresolved and GetPubKey() returns an error. | ||
| func (gs GenesisState) UnpackInterfaces(unpacker codectypes.AnyUnpacker) error { | ||
| for i := range gs.AttesterInfos { | ||
| if err := gs.AttesterInfos[i].UnpackInterfaces(unpacker); err != nil { | ||
| return fmt.Errorf("unpack attester %d: %w", i, err) | ||
| } | ||
| } | ||
| return nil | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check if UnpackInterfaces is called automatically by the codec or if manual invocation is needed
# Search for how other modules handle UnpackInterfaces in InitGenesis
rg -n -A5 "UnpackInterfaces" --type=go -g '!*_test.go' | head -60
# Check the module's InitGenesis implementation
ast-grep --pattern $'func (am AppModule) InitGenesis($_, $_, $_) {
$$$
}'Repository: evstack/ev-abci
Length of output: 4419
Call UnpackInterfaces on genesis state after unmarshaling.
The implementation is correct, but AppModule.InitGenesis (modules/network/module.go:87-93) calls cdc.MustUnmarshalJSON without subsequently invoking UnpackInterfaces on the result. This leaves the Any-typed pubkeys unresolved, causing GetPubKey() to fail. The codebase shows this pattern is necessary elsewhere (e.g., modules/migrationmngr/keeper/migration.go:20, server/migration_cmd.go:509).
Add genesisState.UnpackInterfaces(cdc) after unmarshaling in InitGenesis.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@modules/network/types/genesis.go` around lines 10 - 20, After deserializing
the genesis with cdc.MustUnmarshalJSON in AppModule.InitGenesis
(modules/network/module.go), call the GenesisState.UnpackInterfaces method to
resolve Any-typed pubkeys; specifically, after the existing
cdc.MustUnmarshalJSON into the genesisState variable, invoke
genesisState.UnpackInterfaces(cdc) (or the appropriate AnyUnpacker) and
handle/return any error to ensure AttesterInfo.GetPubKey() will not fail;
reference GenesisState.UnpackInterfaces and InitGenesis when making the change.
Comment on lines
+499
to
+502
| heightStr := blockResp.Result.Block.Header.Height | ||
| if heightStr == "" { | ||
| return 0, nil | ||
| } |
Contributor
There was a problem hiding this comment.
Empty height string returns 0 — could cause polling loop issues.
If the node returns an empty height (e.g., no blocks yet), getLatestHeight returns 0. The caller's for h := nextHeight; h <= currentHeight loop with nextHeight=1 and currentHeight=0 won't execute, which is correct. However, if nextHeight is already > 1 and height drops to 0 (unlikely but possible during restarts), attestations could stall.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/attester_cmd.go` around lines 499 - 502, getLatestHeight currently
returns 0 when blockResp.Result.Block.Header.Height == "" which can silently
stall the caller loop (for h := nextHeight; h <= currentHeight). Change
getLatestHeight to return a sentinel negative value (e.g., -1, nil) when Height
is empty and do not treat that as a valid height; then update the caller that
iterates using currentHeight (the for h := nextHeight; h <= currentHeight loop)
to check if currentHeight <= 0 and skip/poll without attempting to process
ranges in that case. Reference getLatestHeight and
blockResp.Result.Block.Header.Height in your changes and adjust the caller's
loop guard to explicitly handle currentHeight <= 0.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
server/attester_cmd_test.go (1)
54-68: Use malformed node URL in short-circuit tests to prove no parse/network happens.Using
tcp://localhost:26657makes these cases slightly environment-coupled. Passing"%"here better validates that Line 55/63 returns early before URL parsing or RPC.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/attester_cmd_test.go` around lines 54 - 68, Replace the environment-coupled node URL used in the short-circuit tests with a clearly malformed URL so parsing/networking can't occur; specifically, update the two tests that call getOriginalBlockID (the "height 0 returns empty block ID without RPC" and "height 1 returns empty block ID without RPC" cases) to pass "%" instead of "tcp://localhost:26657" as the node URL, keeping the rest of the assertions unchanged to prove the function returns early without attempting RPC or URL parsing.modules/network/keeper/msg_server.go (1)
237-240: Test hook is acceptable but consider naming convention.The test hook pattern is fine for unit testing internal functions. Consider renaming to
VerifyVoteExportedForTestor adding a//go:build testconstraint in a separate file to prevent accidental production usage.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/network/keeper/msg_server.go` around lines 237 - 240, The public test hook VerifyVoteForTest could be accidentally used in production; rename the function to a test-only identifier (e.g., VerifyVoteExportedForTest) and update all references, or move the wrapper into a separate file with a test-only build tag (add a new file containing the wrapper with a top-line "//go:build test" and package same as Keeper) so the wrapper around msgServer.verifyVote (the function calling msgServer{Keeper: k}.verifyVote) is only compiled for tests; ensure you update imports and any unit tests to call the new symbol VerifyVoteExportedForTest or rely on the test-only file.modules/network/keeper/msg_server_test.go (1)
857-883: Consider consolidating with existing test helpers.
signTestVoteAtis very similar tosignTestVoteWithBlockIDintesthelpers_test.go(from context snippet 2). The main difference is the explicit timestamp parameter. Consider either:
- Adding a timestamp parameter to
signTestVoteWithBlockID, or- Creating a builder pattern for test votes
This would reduce duplication across test files.
♻️ Suggested consolidation in testhelpers_test.go
+// signTestVoteWithOptions provides full control over vote construction for tests. +func signTestVoteWithOptions(t *testing.T, chainID string, height int64, priv cmted25519.PrivKey, blockID cmtproto.BlockID, timestamp time.Time) []byte { + t.Helper() + pub := priv.PubKey().(cmted25519.PubKey) + v := cmtproto.Vote{ + Type: cmtproto.PrecommitType, + Height: height, + Round: 0, + BlockID: blockID, + Timestamp: timestamp, + ValidatorAddress: pub.Address(), + ValidatorIndex: 0, + } + sb := cmttypes.VoteSignBytes(chainID, &v) + sig, err := priv.Sign(sb) + require.NoError(t, err) + v.Signature = sig + out, err := proto.Marshal(&v) + require.NoError(t, err) + return out +} + func signTestVote(t *testing.T, chainID string, height int64, priv cmted25519.PrivKey, blockIDHash []byte) []byte { t.Helper() - return signTestVoteWithBlockID(t, chainID, height, priv, cmtproto.BlockID{ + return signTestVoteWithOptions(t, chainID, height, priv, cmtproto.BlockID{ Hash: blockIDHash, PartSetHeader: cmtproto.PartSetHeader{}, - }) + }, testTimeUTC()) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/network/keeper/msg_server_test.go` around lines 857 - 883, The signTestVoteAt helper duplicates signTestVoteWithBlockID; change testhelpers_test.go's signTestVoteWithBlockID to accept an optional time.Time parameter (or overload it by adding a timestamp argument), move the signing logic into that single function, and have signTestVoteAt either be removed or become a thin wrapper that calls signTestVoteWithBlockID(chainID, height, priv, blockIDHash, timestamp). Update all callers to use the unified helper (adjusting signatures/imports) so there is no duplicated vote-signing logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/attester_cmd_test.go`:
- Around line 317-324: The test HTTP mock currently returns the same payload for
any request, allowing wrong paths/queries to pass; modify the httptest.NewServer
handler in the helper to assert the incoming request path and query (e.g., check
r.URL.Path and r.URL.Query().Get("height") or r.URL.RawQuery) and call
t.Fatalf/t.Errorf on mismatch before writing the JSON response so callers that
expect "/block" and specific query params fail fast; update references to the
helper (the server handler closure and any callers relying on
"tcp://"+strings.TrimPrefix(server.URL, "http://")) accordingly.
---
Nitpick comments:
In `@modules/network/keeper/msg_server_test.go`:
- Around line 857-883: The signTestVoteAt helper duplicates
signTestVoteWithBlockID; change testhelpers_test.go's signTestVoteWithBlockID to
accept an optional time.Time parameter (or overload it by adding a timestamp
argument), move the signing logic into that single function, and have
signTestVoteAt either be removed or become a thin wrapper that calls
signTestVoteWithBlockID(chainID, height, priv, blockIDHash, timestamp). Update
all callers to use the unified helper (adjusting signatures/imports) so there is
no duplicated vote-signing logic.
In `@modules/network/keeper/msg_server.go`:
- Around line 237-240: The public test hook VerifyVoteForTest could be
accidentally used in production; rename the function to a test-only identifier
(e.g., VerifyVoteExportedForTest) and update all references, or move the wrapper
into a separate file with a test-only build tag (add a new file containing the
wrapper with a top-line "//go:build test" and package same as Keeper) so the
wrapper around msgServer.verifyVote (the function calling msgServer{Keeper:
k}.verifyVote) is only compiled for tests; ensure you update imports and any
unit tests to call the new symbol VerifyVoteExportedForTest or rely on the
test-only file.
In `@server/attester_cmd_test.go`:
- Around line 54-68: Replace the environment-coupled node URL used in the
short-circuit tests with a clearly malformed URL so parsing/networking can't
occur; specifically, update the two tests that call getOriginalBlockID (the
"height 0 returns empty block ID without RPC" and "height 1 returns empty block
ID without RPC" cases) to pass "%" instead of "tcp://localhost:26657" as the
node URL, keeping the rest of the assertions unchanged to prove the function
returns early without attempting RPC or URL parsing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5d52316e-e425-448d-b397-80d5877bd248
📒 Files selected for processing (3)
modules/network/keeper/msg_server.gomodules/network/keeper/msg_server_test.goserver/attester_cmd_test.go
Comment on lines
+317
to
324
| server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { | ||
| w.Header().Set("Content-Type", "application/json") | ||
| _, _ = w.Write([]byte(response)) | ||
| })) | ||
| t.Cleanup(server.Close) | ||
|
|
||
| return "tcp://" + strings.TrimPrefix(server.URL, "http://") | ||
| } |
Contributor
There was a problem hiding this comment.
Helper should assert request path/query to prevent false-positive tests.
Right now the mock server returns the payload for any request. A regression like calling /status (or wrong query params) could still pass.
Suggested hardening
-func newAttesterRPCTestServer(t *testing.T, response string) string {
+func newAttesterRPCTestServer(t *testing.T, response string, assertReq func(*http.Request)) string {
t.Helper()
- server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+ server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ if assertReq != nil {
+ assertReq(r)
+ }
w.Header().Set("Content-Type", "application/json")
_, _ = w.Write([]byte(response))
}))Then in callers, assert expected path/query (e.g., /block, height=...) per helper under test.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { | |
| w.Header().Set("Content-Type", "application/json") | |
| _, _ = w.Write([]byte(response)) | |
| })) | |
| t.Cleanup(server.Close) | |
| return "tcp://" + strings.TrimPrefix(server.URL, "http://") | |
| } | |
| func newAttesterRPCTestServer(t *testing.T, response string, assertReq func(*http.Request)) string { | |
| t.Helper() | |
| server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | |
| if assertReq != nil { | |
| assertReq(r) | |
| } | |
| w.Header().Set("Content-Type", "application/json") | |
| _, _ = w.Write([]byte(response)) | |
| })) | |
| t.Cleanup(server.Close) | |
| return "tcp://" + strings.TrimPrefix(server.URL, "http://") | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/attester_cmd_test.go` around lines 317 - 324, The test HTTP mock
currently returns the same payload for any request, allowing wrong paths/queries
to pass; modify the httptest.NewServer handler in the helper to assert the
incoming request path and query (e.g., check r.URL.Path and
r.URL.Query().Get("height") or r.URL.RawQuery) and call t.Fatalf/t.Errorf on
mismatch before writing the JSON response so callers that expect "/block" and
specific query params fail fast; update references to the helper (the server
handler closure and any callers relying on
"tcp://"+strings.TrimPrefix(server.URL, "http://")) accordingly.
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.
Summary
genesis) signs each block's
cmtproto.Vote; the ABCI adapter reconstructs acmttypes.Committhat a 07-tendermint light client verifies viaVerifyCommitLighton counterparty chains.MsgAttestand pinvote.BlockID.Hashto the sequencer's stored block hash, preventingattester-forged-BlockID attacks.
pkg/rpc/core/blocks.go: ordered byvalidator address,
BlockIDFlagAbsentfor non-signers, 2/3 voted-powerquorum gate.
server/attester_cmd.go) signs every block with theed25519 consensus key, derives the consensus address correctly, and fetches
latest height via
/block(not/status, which returns 0 in attester mode).MsgJoinAttesterSet/MsgLeaveAttesterSet; the set isgenesis-fixed for this iteration.
VerifyCommitLightpasses on an attested commit and exercises bidirectionalICS20 transfers.
Notable details
GenesisState.UnpackInterfacesimplemented soAny-typed pubkeys resolveafter JSON unmarshaling.
(prefix-agnostic bech32) to tolerate celestia/cosmos prefix drift.
BlockIDProviderwired post-adapter-creation via the app-sidenetworkKeeperBlockIDWirerinterface; the integration-testapp.gopatchinjects
SetNetworkKeeperBlockIDProviderautomatically.Known limitations
Celestia v4's
tokenfilterIBC middleware rejects non-utia inbound —counterparty policy, not an attester-system bug. Follow-up: convert to
assert the expected ack-error, or revert.
(each attester stamps votes from its own
/blockread) is not exercised.couple of tests, `GenesisState.Validate()` doesn't check `AttesterInfos`,
CLI still registers disabled tx subcommands. Non-blocking for this draft.
Summary by CodeRabbit
New Features
Bug Fixes
Tests