Set Rate Limit Admin #1101

agusaldasoro · 2025-07-23T14:43:22Z

Expose a message to modify the Rate Limit Admin in the Token Pool

agusaldasoro · 2025-07-23T14:43:44Z

chains/solana/contracts/programs/base-token-pool/src/common.rs

@@ -187,6 +187,25 @@ impl BaseConfig {
        self.can_accept_liquidity = allow;
        Ok(())
    }
+
+    pub fn set_rate_limit_admin(&mut self, new_rate_limit_admin: Pubkey) -> Result<()> {
+        require_keys_neq!(


Is this expected?

toblich

Could you please add a basic integration test for the new method?

chains/solana/contracts/programs/lockrelease-token-pool/src/context.rs

toblich · 2025-07-23T20:38:40Z

chains/solana/contracts/programs/cctp-token-pool/src/context.rs

+        ],
+        bump,
+    )]
+    pub state: Account<'info, State>,


Please also check the state version here (same comment applies to other contexts)

toblich · 2025-07-23T20:40:12Z

chains/solana/contracts/programs/burnmint-token-pool/src/context.rs

        seeds = [POOL_STATE_SEED, mint.key().as_ref()],
        bump,
    )]
    pub state: Account<'info, State>,
-    #[account(mut, constraint = authority.key() == state.config.owner)]
+    #[account(mut, constraint = authority.key() == state.config.owner @ CcipTokenPoolError::Unauthorized)]


Nitpick

Suggested change

#[account(mut, constraint = authority.key() == state.config.owner @ CcipTokenPoolError::Unauthorized)]

#[account(mut, address = state.config.owner @ CcipTokenPoolError::Unauthorized)]

Same comment applies to other address checks

toblich · 2025-07-23T20:43:02Z

chains/solana/contracts/tests/ccip/tokenpool_test.go

@@ -454,6 +454,56 @@ func TestTokenPool(t *testing.T) {
 							}
 						})

+						t.Run("Rate Limit Admin", func(t *testing.T) {
+							// set invalid rate limit admin


Fix comment plz

toblich · 2025-07-23T20:43:23Z

chains/solana/contracts/tests/ccip/tokenpool_test.go

+
+							testutils.SendAndConfirm(ctx, t, solanaGoClient, []solana.Instruction{ixRatesValid}, user, config.DefaultCommitment)
+
+							// set invalid rate limit admin


fix comment plz

toblich · 2025-07-23T20:45:48Z

chains/solana/contracts/tests/ccip/tokenpool_test.go

+							).ValidateAndBuild()
+							require.NoError(t, err)
+
+							testutils.SendAndConfirm(ctx, t, solanaGoClient, []solana.Instruction{ixRateAdmin}, anotherAdmin, config.DefaultCommitment)


Just a suggestion: Consider packing some of these ixs in a single tx, so that it has fewer side-effects that can break other parallel tests. For example, maybe a single tx could do

Set user as RL admin

user sets low RL value

owner sets back original RL value (or slightly different one even)

Thus, as the tx is atomic, no concurrent test will face issues. And you can check in the intermediate events and in the final state that everything worked as expected.

github-actions · 2025-07-24T19:08:08Z

Metric	`feat/ratelimiter-admin`	`main`
Coverage	70.0%	69.6%

toblich

Approved, though if you have time to improve the test please do so (see comment)

toblich · 2025-07-27T22:17:23Z

chains/solana/contracts/tests/ccip/tokenpool_test.go

+								}, poolConfig, p.Chain[config.EvmChainSelector], user.PublicKey(), solana.SystemProgramID).ValidateAndBuild()
+							require.NoError(t, err)
+
+							testutils.SendAndFailWith(ctx, t, solanaGoClient, []solana.Instruction{ixRateAdmin, ixRatesValid, ixRateAdmin2, ixRates}, user, config.DefaultCommitment, []string{"SetChainRateLimit"}, common.AddSigners(anotherAdmin))


That []string{"SetChainRateLimit"} only checks that that string is present in the logs, meaning that it will only check that the method was invoked. It won't enforce that that is the method that's failing, nor which of the two invocations you're making to is the one that fails.

I'll approve the PR anyway as I think this is minor compared to other priorities, but if you have the time to improve this check, please do so 🙂. You could check the events to make sure that the first call to that method did work for example.

toblich · 2025-07-27T22:19:16Z

chains/solana/contracts/programs/cctp-token-pool/src/context.rs

+    )]
+    pub chain_config: Account<'info, ChainConfig>,
+
+    #[account(mut, constraint = authority.key() == state.config.owner @ CcipTokenPoolError::Unauthorized)]


Nitpick

Suggested change

#[account(mut, constraint = authority.key() == state.config.owner @ CcipTokenPoolError::Unauthorized)]

#[account(mut, address = state.config.owner @ CcipTokenPoolError::Unauthorized)]

toblich · 2025-07-27T22:20:51Z

chains/solana/contracts/programs/cctp-token-pool/src/context.rs

@@ -638,7 +673,21 @@ pub struct EditChainConfig<'info> {
    )]
    pub chain_config: Account<'info, ChainConfig>,

-    #[account(mut, constraint = authority.key() == state.config.owner || authority.key() == state.config.rate_limit_admin)]


Thanks a lot for finding this and fixing it 🫶

* Set Rate Limit Admin * Add tests for set rate limit admin * Fix comment in test * Send all instructions in the same transaction * Validate State version * Improve validation

Set Rate Limit Admin

f094de8

agusaldasoro commented Jul 23, 2025

View reviewed changes

agusaldasoro requested a review from toblich July 23, 2025 15:38

agusaldasoro marked this pull request as ready for review July 23, 2025 15:38

agusaldasoro requested a review from a team as a code owner July 23, 2025 15:38

toblich reviewed Jul 23, 2025

View reviewed changes

chains/solana/contracts/programs/lockrelease-token-pool/src/context.rs Show resolved Hide resolved

agusaldasoro added 2 commits July 23, 2025 16:32

Add tests for set rate limit admin

cb7c9d6

Fix comment in test

f7d4c91

toblich reviewed Jul 23, 2025

View reviewed changes

agusaldasoro added 3 commits July 24, 2025 10:08

Send all instructions in the same transaction

50a6905

Validate State version

be84329

Improve validation

dce93ff

toblich approved these changes Jul 27, 2025

View reviewed changes

agusaldasoro enabled auto-merge July 28, 2025 09:10

PabloMansanet approved these changes Jul 28, 2025

View reviewed changes

agusaldasoro added this pull request to the merge queue Jul 28, 2025

Merged via the queue into main with commit 4fd7b48 Jul 28, 2025
53 of 54 checks passed

mateusz-sekara deleted the feat/ratelimiter-admin branch July 28, 2025 11:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Set Rate Limit Admin #1101

Set Rate Limit Admin #1101

Uh oh!

agusaldasoro commented Jul 23, 2025 •

edited

Loading

Uh oh!

agusaldasoro Jul 23, 2025

Uh oh!

toblich left a comment

Uh oh!

Uh oh!

toblich Jul 23, 2025

Uh oh!

toblich Jul 23, 2025

Uh oh!

toblich Jul 23, 2025

Uh oh!

toblich Jul 23, 2025

Uh oh!

toblich Jul 23, 2025

Uh oh!

github-actions bot commented Jul 24, 2025

Uh oh!

toblich left a comment

Uh oh!

toblich Jul 27, 2025

Uh oh!

toblich Jul 27, 2025

Uh oh!

toblich Jul 27, 2025

Uh oh!

Uh oh!

Uh oh!

	#[account(mut, constraint = authority.key() == state.config.owner @ CcipTokenPoolError::Unauthorized)]
	#[account(mut, address = state.config.owner @ CcipTokenPoolError::Unauthorized)]


		testutils.SendAndConfirm(ctx, t, solanaGoClient, []solana.Instruction{ixRatesValid}, user, config.DefaultCommitment)

		// set invalid rate limit admin

Set Rate Limit Admin #1101

Set Rate Limit Admin #1101

Uh oh!

Conversation

agusaldasoro commented Jul 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

toblich left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jul 24, 2025

Uh oh!

toblich left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

agusaldasoro commented Jul 23, 2025 •

edited

Loading