Add set rate limits#22503
Conversation
|
✅ No conflicts with other open PRs targeting |
agusaldasoro
requested review from
asoliman92,
b-gopalswami,
carte7000,
makramkd,
ogtownsend,
tt-cll and
winder
product-security-plaid-production
Bot
requested review from
ChrisAmora and
patrickhuie19
| if entry.PoolType == "" { | ||
| return errors.New("pool type must be defined") | ||
| } | ||
| if entry.RemoteChainSelector == 0 { | ||
| return errors.New("remote chain selector must be defined") | ||
| } |
There was a problem hiding this comment.
Are we checking somewhere that they are also valid?
There was a problem hiding this comment.
the remote chain selector you mean? Yes, in line 3296 isSup, _, err := isSupportedChain(chain, tokenPubKey, tokenPool, entry.PoolType, entry.RemoteChainSelector)
| MCMS *proposalutils.TimelockConfig | ||
| } | ||
|
|
||
| func (cfg SetChainRateLimitConfig) Validate(e cldf.Environment, chainState solanastateview.CCIPChainState) error { |
There was a problem hiding this comment.
Would it be worth checking as well that our keys (deployer or timelock) are the RL admin? So that no one attempts to execute this on a pool that has been handed over to a customer, and gets their root committed to MCMS but cannot execute later the actual RL update.
| tokenPoolUsingMcms := solanastateview.IsSolanaProgramOwnedByTimelock( | ||
| &e, | ||
| chain, | ||
| chainState, | ||
| entry.PoolType, | ||
| tokenPubKey, | ||
| entry.Metadata, | ||
| ) | ||
| authority := GetAuthorityForIxn( | ||
| &e, | ||
| chain, | ||
| chainState, | ||
| entry.PoolType, | ||
| tokenPubKey, | ||
| entry.Metadata, | ||
| ) |
There was a problem hiding this comment.
Aren't both checks doing pretty much the same thing? At this point AFAIK there should be only 2 options, either deployer and not MCMS, or using MCMS and the authority is timelock, right?
There was a problem hiding this comment.
Pull request overview
Risk Rating: MEDIUM
Adds a Solana v0.1.1 changeset for updating rate limits on existing token pool remote-chain configs.
Changes:
- Registers
SetChainRateLimitas a changeset. - Adds config/validation types for per-chain token pool rate-limit updates.
- Builds direct Solana instructions or MCMS proposal transactions, including a dummy-rate workaround.
Targeted Human Review Areas:
- MCMS proposal generation and validation around
cfg.MCMS. - Dummy-rate workaround logic for disabled-to-enabled rate-limit transitions.
- Missing test coverage for the new changeset paths.
|
|
||
| if len(mcmsTxs) > 0 { | ||
| proposal, err := BuildProposalsForTxns( | ||
| e, cfg.SolChainSelector, "proposal to SetChainRateLimit in Solana", cfg.MCMS.MinDelay, mcmsTxs) |
| // If the account doesn't exist yet (e.g. during token expansion where MCMS batches | ||
| // init_chain_remote_config and set_chain_rate_limit in a single proposal), we treat | ||
| // both directions as uninitialized and always prepend the dummy instruction. |
| return nil | ||
| } | ||
|
|
||
| func SetChainRateLimit(e cldf.Environment, cfg SetChainRateLimitConfig) (cldf.ChangesetOutput, error) { |
|
Create changeset to modify rate limits of an existing pool in solana