Skip to content

Add precompile-backed bank query client#3424

Open
codchen wants to merge 7 commits into
mainfrom
codex/precompile-backed-query-clients-bank
Open

Add precompile-backed bank query client#3424
codchen wants to merge 7 commits into
mainfrom
codex/precompile-backed-query-clients-bank

Conversation

@codchen
Copy link
Copy Markdown
Collaborator

@codchen codchen commented May 13, 2026

Summary

  • add a shared precompiles/query ClientConn bridge that maps generated QueryClient calls to EVM RPC eth_call precompile bindings
  • add bank query bindings and wire bank CLI queries through the precompile-backed generated client
  • extend the bank precompile with missing retained query methods and document the migration proposal in RFC-002

Tests

  • go test ./precompiles/...
  • go test ./precompiles/query ./precompiles/bank/query ./precompiles/bank ./precompiles/addr ./sei-cosmos/x/bank/client/cli

Note

Medium Risk
Introduces a new query transport layer that routes generated gRPC QueryClient calls through EVM RPC eth_call, and expands the bank precompile ABI; errors or ABI mismatches could break CLI/query behavior despite added tests.

Overview
Switches bank CLI queries to EVM RPC. x/bank CLI commands now build their generated QueryClient on top of a new precompiles/query ClientConn, configured via a new --evm-rpc flag (default http://localhost:8545) and honoring the existing height selection.

Adds a shared precompile-backed query bridge + bank bindings. New precompiles/query code provides a registry-based method router, ABI pack/unpack helpers, and Sei↔EVM address conversion via the addr precompile; precompiles/bank/query registers bindings for key bank queries (balances, spendable, supply, params, metadata) including pagination reconstruction.

Extends the bank precompile query surface. The bank precompile ABI and executor add missing retained view methods (string-address variants, spendable balances, total supply, denom metadata, params) and refactors supply handling; interfaces and tests are updated accordingly, plus an RFC documenting the migration approach.

Reviewed by Cursor Bugbot for commit d750c83. Bugbot is set up for automated code reviews on this repo. Configure here.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 13, 2026

The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).

BuildFormatLintBreakingUpdated (UTC)
✅ passed✅ passed✅ passed✅ passedMay 18, 2026, 3:50 AM

@codecov
Copy link
Copy Markdown

codecov Bot commented May 13, 2026

Codecov Report

❌ Patch coverage is 67.65101% with 241 lines in your changes missing coverage. Please review.
✅ Project coverage is 59.35%. Comparing base (823a78d) to head (279b713).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
precompiles/bank/query/registry.go 68.35% 41 Missing and 40 partials ⚠️
precompiles/bank/bank.go 70.55% 24 Missing and 24 partials ⚠️
precompiles/query/helpers.go 68.80% 17 Missing and 17 partials ⚠️
precompiles/query/binding.go 46.51% 14 Missing and 9 partials ⚠️
precompiles/query/address.go 64.28% 10 Missing and 10 partials ⚠️
precompiles/query/conn.go 74.57% 11 Missing and 4 partials ⚠️
sei-cosmos/x/bank/client/cli/query.go 78.26% 5 Missing and 5 partials ⚠️
sei-cosmos/x/bank/client/testutil/suite.go 0.00% 9 Missing ⚠️
sei-cosmos/x/bank/client/testutil/cli_helpers.go 0.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3424      +/-   ##
==========================================
+ Coverage   59.29%   59.35%   +0.06%     
==========================================
  Files        2125     2130       +5     
  Lines      175629   176358     +729     
==========================================
+ Hits       104144   104684     +540     
- Misses      62404    62474      +70     
- Partials     9081     9200     +119     
Flag Coverage Δ
sei-chain-pr 51.50% <67.65%> (?)
sei-db 70.41% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
precompiles/addr/addr.go 67.66% <100.00%> (+0.49%) ⬆️
precompiles/utils/expected_keepers.go 82.35% <ø> (ø)
sei-cosmos/x/bank/client/testutil/cli_helpers.go 0.00% <0.00%> (ø)
sei-cosmos/x/bank/client/testutil/suite.go 0.00% <0.00%> (ø)
sei-cosmos/x/bank/client/cli/query.go 81.60% <78.26%> (+30.46%) ⬆️
precompiles/query/conn.go 74.57% <74.57%> (ø)
precompiles/query/address.go 64.28% <64.28%> (ø)
precompiles/query/binding.go 46.51% <46.51%> (ø)
precompiles/query/helpers.go 68.80% <68.80%> (ø)
precompiles/bank/bank.go 66.85% <70.55%> (+4.83%) ⬆️
... and 1 more
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread precompiles/query/address.go
Comment thread sei-cosmos/x/bank/client/cli/query.go
Copy link
Copy Markdown
Collaborator Author

codchen commented May 13, 2026

Pushed 586d16bf6 to address the CI failures I could reproduce from the logs:

  • Added string-address bank precompile query entrypoints for Balance, AllBalances, and SpendableBalances, so CLI/generated bank queries no longer require an addr-precompile EVM association for Bech32 account addresses.
  • Kept the existing address-based contract-facing bank methods intact.
  • Annotated the pagination conversions that triggered gosec G115 after explicit bounds checks.

Local verification:

go test ./precompiles/query ./precompiles/bank/query ./precompiles/bank ./precompiles/addr ./sei-cosmos/x/bank/client/cli
go test ./precompiles/...

The repo golangci-lint command did not run locally because the linter binary reports it was built with Go 1.24 while the repo targets Go 1.25.6.

Comment thread sei-cosmos/x/bank/client/cli/query.go Outdated
Comment thread precompiles/query/binding.go Outdated
Comment thread precompiles/bank/query/registry.go Outdated
pageRes := &sdkquery.PageResponse{NextKey: nextKey}
if countTotal && !keyPagination {
pageRes.Total = orderedLen
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pagination omits Total for key-based requests

Medium Severity

The paginate function deliberately skips setting pageRes.Total when key-based pagination is used (countTotal && !keyPagination), but the Cosmos SDK's Paginate sets Total for key-based pagination when CountTotal is true. Since the bindings (e.g., AllBalances, SpendableBalances, TotalSupply) claim ExactProtobufShape, consumers relying on Total being populated during key-based paginated queries will get a zero value instead of the actual item count, silently diverging from the original gRPC query behavior.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 93b0b77. Configure here.

Copy link
Copy Markdown
Collaborator Author

codchen commented May 13, 2026

Pushed d750c8362 to optimize bank precompile total_supply accumulation. It now appends positive coins from the already-ordered total supply iterator instead of calling sdk.Coins.Add on every item, avoiding the O(n²) behavior.

Local verification:

go test ./precompiles/bank ./precompiles/bank/query ./precompiles/query ./sei-cosmos/x/bank/client/cli

@codchen codchen force-pushed the codex/precompile-backed-query-clients-bank branch from d750c83 to b982569 Compare May 18, 2026 03:25
@cursor
Copy link
Copy Markdown

cursor Bot commented May 18, 2026

PR Summary

Medium Risk
Switches x/bank CLI query execution to EVM JSON-RPC eth_call via a new gogogrpc.ClientConn bridge and expands the bank precompile ABI, which could affect query correctness/pagination and introduces reliance on an EVM RPC endpoint.

Overview
Adds a reusable precompile-backed query transport under precompiles/query that implements gogogrpc.ClientConn and routes generated protobuf QueryClient calls to EVM JSON-RPC eth_call via per-module binding registries (with shared ABI decoding, pagination, and address-conversion helpers).

Implements the first module bindings for bank in precompiles/bank/query, mapping core bank gRPC query methods (balances, supply, params, denom metadata) onto bank precompile ABI methods and reconstructing protobuf responses (including Cosmos-style pagination) from ABI outputs.

Extends the bank precompile query surface (Solidity interface, abi.json, and Go executor) with additional retained view methods (bech32-address variants, spendable balances, total supply, denom metadata, params) and refactors helpers for coin/metadata/params encoding.

Updates bank CLI queries to default to the precompile backend, adding --evm-rpc plus a hidden backend switch for parity testing, and introduces integration/unit tests validating legacy vs precompile output parity and height handling; also adds RFC-002 documenting the migration approach.

Reviewed by Cursor Bugbot for commit 279b713. Bugbot is set up for automated code reviews on this repo. Configure here.

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

There are 2 total unresolved issues (including 1 from previous review).

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 279b713. Configure here.

}
if req.Offset > 0 && req.Key != nil {
return nil, nil, fmt.Errorf("invalid request, either offset or key is expected, got both")
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Paginate offset/key check diverges from Cosmos SDK

High Severity

The Paginate function checks req.Offset > 0 && req.Key != nil to reject combined offset+key pagination, but the Cosmos SDK checks req.Offset > 0 && len(req.Key) > 0. Since ReadPageRequest constructs Key: []byte(pageKey), an unset --page-key flag produces []byte{} (not nil, length 0). Using --offset=N without --page-key will error on the precompile backend but succeed on the legacy backend, breaking parity for offset-based pagination.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 279b713. Configure here.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant