feat(ccq-server): enforce rate limits based on staking #4632
Conversation
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
aapclark
commented
Jan 6, 2026
aapclark
force-pushed
the
ccq-server-staking-rate-limits
branch
from
cabf87e to
81ac7e8
Compare
djb15
reviewed
Jan 26, 2026
Collaborator
djb15
left a comment
djb15
left a comment
There was a problem hiding this comment.
Passing on some initial smaller comments
node/pkg/query/request.go
Outdated
| if version == MSG_VERSION_V2 { | ||
| stakerLen := uint8(len(queryRequest.StakerAddress)) // #nosec G115 -- `StakerAddress` length checked in `Validate` | ||
| vaa.MustWrite(buf, binary.BigEndian, stakerLen) | ||
| if stakerLen > 0 { |
Collaborator
There was a problem hiding this comment.
This stakerLen check is superfluous as it is checked above. Although it seems like we're generally overloading MSG_VERSION_V2 to stakerLen > 0, I would prefer not overloading based on the length of an input and instead being more explicit about the message version?
node/pkg/query/querystaking/evm.go
Outdated
| } | ||
|
|
||
| // ParseSignerAddress parses the result of stakerSigners(address) call. | ||
| func ParseSignerAddress(data []byte) (common.Address, error) { |
Collaborator
There was a problem hiding this comment.
Can you also add a check that the upper 12 bytes are 0?
| } | ||
|
|
||
| // ParseBoolResult parses the result of a bool-returning contract call. | ||
| func ParseBoolResult(data []byte) (bool, error) { |
Collaborator
There was a problem hiding this comment.
Same here, let's check all the upper bytes are 0 to ensure we're actually parsing a boolean
node/pkg/query/querystaking/evm.go
Outdated
| } | ||
|
|
||
| // ParseAddressResult parses an address result from a contract call. | ||
| func ParseAddressResult(data []byte) (common.Address, error) { |
Collaborator
There was a problem hiding this comment.
Same point re checking upper bytes
| } | ||
|
|
||
| // ParseUint8Result parses a uint8 result from a contract call. | ||
| func ParseUint8Result(data []byte) (uint8, error) { |
Collaborator
There was a problem hiding this comment.
Same point re checking upper bytes
node/pkg/query/querystaking/pools.go
Outdated
| return common.Address{}, fmt.Errorf("failed to call queryTypePools on factory: %w", err) | ||
| } | ||
|
|
||
| poolAddress, err := ParseSignerAddress(result) // Reuse the address parsing logic |
Collaborator
There was a problem hiding this comment.
We have ParseAddressResult available too, probably get rid of ParseSignerAddress for the more generically named variant?
| ) | ||
|
|
||
| // parseAllowedRequesters parses a comma-separated list of allowed requesters for testing | ||
| func parseAllowedRequesters(allowedRequesters string) (map[ethCommon.Address]struct{}, error) { |
Collaborator
There was a problem hiding this comment.
Instead of adding in this helper function can you instead remove all the tests that use the previous requester whitelisting logic?
Collaborator
There was a problem hiding this comment.
Please add some test cases for the changes in this file
node/pkg/query/request.go
Outdated
| // MSG_VERSION is the current version of the CCQ message protocol. | ||
| const MSG_VERSION uint8 = 1 | ||
| const ( | ||
| // MSG_VERSION is the current version of the CCQ message protocol. |
Collaborator
There was a problem hiding this comment.
Suggested change
| // MSG_VERSION is the current version of the CCQ message protocol. | |
| // MSG_VERSION is the original version of the CCQ message protocol. |
node/pkg/query/querystaking/ipfs.go
Outdated
| // parseRateString parses rate strings like "1 QPS" or "5 QPM" into queries per minute | ||
| // QPS (queries per second) is converted to QPM by multiplying by 60 | ||
| // QPM (queries per minute) is used directly | ||
| func parseRateString(rateStr string) (uint64, error) { |
Collaborator
There was a problem hiding this comment.
I don't think this function is used anywhere?
Rate limiting is enforced by the CCQ server before requests reach the guardian. Guardians trust that requests on the gossip network have already been validated. This aligns with the CCQ whitepaper: "The CCQ query server does request validation and responds with an error if it detects a bad request." - (ccq-server): query CCQ staking contract and look up rate limits in IPFS tranche - (ccq-server): support raw and EIP-191 signed messages via X-Signature-Format header - (ccq-server): add config values for contract address and IPFS host - ci: add IPFS provider with staking tranches - ci: add queries staking contract deployment and pool creation - (query): validate hash of data read from IPFS - (ccq-server): add `stakingPoolAddresses` config option (replaces factory contract iteration) - (ccq-server): make `ccqFactoryAddress` config flag optional - (ccq-server): add timestamp gating and signed timestamp support - (ccq-server): check query request signature before unmarshaling and validation - (ccq-server): drop duplicate requests - (ccq-server): add encoding validation and avoid re-marshaling query payload - (js-query): switch from web3 to newer version of viem - (query): prevent signature malleability by enforcing single form - (query): increase sig[v] check strictness - ci: ignore eth-node in check-docker-pin.sh (built locally, not hosted) - (js-query): add integration tests for staking-based rate limits - (js-query): add integration tests for raw and EIP-191 signed messages - (js-query): improve nonce handling for parallel integration tests Co-authored-by: web3ashlee <[email protected]>
aapclark
force-pushed
the
ccq-server-staking-rate-limits
branch
from
656b490 to
56c4124
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Rate limiting is enforced by the CCQ server before requests reach the guardian. Guardians trust that requests on the gossip network have already been validated. This aligns with the CCQ whitepaper: "The CCQ query server does request validation and responds with an error if it detects a bad request."
(ccq-server): support raw and eip191 signed messages
(ccq-server): remove unused permissions file
(ccq-server): add config values for contract address and ipfs host
(ccq-server): add X-Signature-Format header (values: "eip191" or default raw)
(ci): add ipfs provider with staking tranches
(ci): add queries staking contract deployment and pool creation
(js-query): switches to newer version of viem from web3
(js-query): add integration tests for staking-based rate limits
(js-query): add integration tests for raw and eip191 signed messages