feat: whitelist and unwhitelist SPL tokens #41

brewmaster012 · 2024-10-08T03:31:22Z

closes #42

Add whitelisting function to SPL tokens to be deposited into the program PDA.
Right now the authority are authorized to whitelist/de-whitelist SPL token.
The integration tests are also refactored somewhat to increase code reuse.

Only deposit_spl_token*() instructions need to provide evidence of whitelisting.
withdraw_spl_token is not encumbered by whitelisting for the following reasons:

when the SPL token has never been whitelisted, there is no way to deposit into the program therefore
no way to withdraw; there is no need for witheraw_spl_token to respect whitelisting;
when the SPL token was whitelisted and later de-whitelisted, the desired behavior would be to
pause deposit, and allow withdraw otherwise the deposited SPL tokens will be stuck.

Whitelisting is implemented as a special PDA for each mint account (SPL token id).

#[derive(Accounts)]
pub struct AddToWhitelist<'info> {
    #[account(
        init,
        space=8,
        payer=authority,
        seeds=[
            b"whitelist",
            whitelist_candidate.key().as_ref()
        ],
        bump
    )]
    pub whitelist_entry: Account<'info, WhitelistEntry>,
    pub whitelist_candidate: Account<'info, Mint>,

    #[account(mut, seeds = [b"meta"], bump, has_one = authority)]
    pub pda: Account<'info, Pda>,
    #[account(mut)]
    pub authority: Signer<'info>,

    pub system_program: Program<'info, System>,
}

Whitelisting creates a PDA derived from the mint_account of the whitelisted SPL token.
When a user deposits the whitelisted token, she must provide evidence, i.e. provide the PDA whitelist entry
in the accounts:

#[derive(Accounts)]
pub struct DepositSplToken<'info> {
    #[account(mut)]
    pub signer: Signer<'info>,

    #[account(seeds = [b"meta"], bump)]
    pub pda: Account<'info, Pda>,

    #[account(seeds=[b"whitelist", mint_account.key().as_ref()], bump)]
    pub whitelist_entry: Account<'info, WhitelistEntry>, // attach whitelist entry to show the mint_account is whitelisted

    pub mint_account: Account<'info, Mint>,

    pub token_program: Program<'info, Token>,

    #[account(mut)]
    pub from: Account<'info, TokenAccount>, // this must be owned by signer; normally the ATA of signer
    #[account(mut)]
    pub to: Account<'info, TokenAccount>, // this must be ATA of PDA
}

This way, the existence of the PDA whitelist_entry is the evidence of the mint_account being whitelisted.

De-whitelisting closes the PDA whitelist_entry so that deposit spl token will error with AccountNotInitialized.

query whitelisted status

To query whether a SPL token is whitelisted by the program off-chain, derive the whitelist_entry PDA account
from seeds [b"whitelist", mint_account.key().as_ref()] and access the PDA account. The existence of this
PDA account means whitelisted; otherwise not whitelisted.

To query whether a SPL token is whitelisted by the program on-chain, add evidence whitelist_entry

 #[account(seeds=[b"whitelist", mint_account.key().as_ref()], bump)]
    pub whitelist_entry: Account<'info, WhitelistEntry>, // attach whitelist entry to show the mint_account is whitelisted

    pub mint_account: Account<'info, Mint>,

to the accounts of the instruction. Anchor constraints ensures that whitelist_entry must exists (and initialized)
which means the referenced mint_account must be whitelisted.

testing

both positive and negative tests are in the integration test suite.

$ anchor test

Co-authored-by: skosito <[email protected]>

codecov-commenter · 2024-10-08T03:33:29Z

Codecov Report

Attention: Patch coverage is 33.33333% with 6 lines in your changes missing coverage. Please review.

Project coverage is 9.16%. Comparing base (74c9d81) to head (75ac812).

Files with missing lines	Patch %	Lines
programs/protocol-contracts-solana/src/lib.rs	33.33%	6 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff            @@
##            main     #41      +/-   ##
========================================
+ Coverage   8.30%   9.16%   +0.85%     
========================================
  Files          1       1              
  Lines        253     262       +9     
========================================
+ Hits          21      24       +3     
- Misses       232     238       +6

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

lumtis · 2024-10-08T07:59:35Z

programs/protocol-contracts-solana/src/lib.rs

+    // whitelisting SPL tokens
+    pub fn whitelist_spl_mint(_ctx: Context<AddToWhitelist>) -> Result<()> {
+        Ok(())
+    }
+
+    pub fn de_whitelist_spl_mint(_ctx: Context<DeleteFromWhitelist>) -> Result<()> {
+        Ok(())
+    }


Unclear what these are for

i think all logic for this is in the accounts definition, when you whitelist new one is created (with init), when you unwhitelist its closed (with close)

lumtis · 2024-10-08T08:00:21Z

programs/protocol-contracts-solana/src/lib.rs

@@ -414,6 +428,51 @@ pub struct UpdatePaused<'info> {
    pub signer: Signer<'info>,
 }

+#[derive(Accounts)]
+pub struct AddToWhitelist<'info> {


We should use consistent terminology with Solidity contracts:

whitelist

unwhitelist

changed to whitelist/unwhitelist across the board
e905e67

lumtis · 2024-10-08T08:01:02Z

tests/protocol-contracts-solana.ts

@@ -19,6 +19,56 @@ const keyPair = ec.keyFromPrivate('5b81cdf52ba0766983acf8dd0072904733d92afe4dd34

 const usdcDecimals = 6;

+async function mintSPLToken(conn: anchor.web3.Connection, wallet: anchor.web3.Keypair, mint: anchor.web3.Keypair) {


I think we should remove all console.log in the test, logging in tests should be minimal by default, should printing passing tests

disabled console.log in the test
e905e67

not sure if its ok to overwrite console.log like this, i think the point was to just remove all console.logs calls as they are just useful while tests are being written or during debugging

if there is console.log that is absolutely needed, it is probably better to replace it with assertion

lumtis · 2024-10-08T10:51:48Z

programs/protocol-contracts-solana/src/lib.rs

+    #[account(seeds=[b"whitelist", mint_account.key().as_ref()], bump)]
+    pub whitelist_entry: Account<'info, WhitelistEntry>, // attach whitelist entry to show the mint_account is whitelisted
+
+    pub mint_account: Account<'info, Mint>,


What does mint mean?

There should be no minting involved since we lock/unlock SPL tokens?

according to description Whitelisting is implemented as a special PDA for each mint account (SPL token id). is needed to create pda for mint account and existence of that pda marks that token is whitelisted

What does mint mean?

There should be no minting involved since we lock/unlock SPL tokens?

mint (short for mint account) is the Solana equivalent for ERC20 contract address.
This is Solana jargon.

skosito · 2024-10-08T12:04:17Z

programs/protocol-contracts-solana/src/lib.rs

@@ -423,6 +482,9 @@ pub struct Pda {
    deposit_paused: bool,
 }

+#[account]
+pub struct WhitelistEntry {}


just curious if is there a pros/cons for whitelisting in 2 following ways:

init/close account

store flag within pda marking if its whitelisted or not

is it because having long lived pda in solana is not efficient if not needed because of rent, or something else?

For whitelisting we really need is a mapping like Ethereum address=>bool

But Solana does not have mapping as a data structure like Ethereum mapping.
You'll need to roll your own. Say you want to create a mapping data account
and then store a hashmap or list in the PDA to implement the mapping.
There are two issues:

size of the data structure and therefore the account size could dynamically change;

hard to get deterministic constant access time (key-value lookup)

The PDA for each key-value pair is the idiomatic way to do Ethereum mapping on Solana.
Think of part of the seed as the key (mint account address), and value is the PDA account.

good explanation

ws4charlie · 2024-10-08T16:15:50Z

programs/protocol-contracts-solana/src/lib.rs

+    #[account(mut, seeds = [b"meta"], bump, has_one = authority)]
+    pub pda: Account<'info, Pda>,
+    #[account(mut)]
+    pub authority: Signer<'info>,


It means the signer (the authority) of the Whitelist instruction must be one of the fields in pda struct?

yes,
the b"meta" PDA contains the authority.

ws4charlie

looks good

skosito · 2024-10-08T17:28:31Z

README.md

@@ -1,6 +1,7 @@

 **Note**: Mainnet-beta, testnet, devnet gateway program address:

+Mainnet-beta, testnet, devnet gateway program address: 


seems like changes in this file are from merge with main, probably should be reverted

skosito · 2024-10-08T17:32:53Z

tests/protocol-contracts-solana.ts

@@ -19,6 +19,56 @@ const keyPair = ec.keyFromPrivate('5b81cdf52ba0766983acf8dd0072904733d92afe4dd34

 const usdcDecimals = 6;

+async function mintSPLToken(conn: anchor.web3.Connection, wallet: anchor.web3.Keypair, mint: anchor.web3.Keypair) {


not sure if its ok to overwrite console.log like this, i think the point was to just remove all console.logs calls as they are just useful while tests are being written or during debugging

if there is console.log that is absolutely needed, it is probably better to replace it with assertion

brewmaster012 · 2024-10-08T18:15:52Z

removed the console logs in the next PR. Don't want to create merging issue later so maybe let's put the console logs aside for this one.

brewmaster012 and others added 8 commits October 7, 2024 14:29

add PDA address to README

9555f86

enhance pda validation

46cecb2

Update programs/protocol-contracts-solana/src/lib.rs

f156723

Co-authored-by: skosito <[email protected]>

Update README.md

64e914f

Co-authored-by: skosito <[email protected]>

add pda_ata validation as constraints

b0d3184

WIP: add whitelist func

03d757c

WIP: add whitelist tag to DepositSplToken

b20cf35

add de-whitelist and its test

b7a4d63

fmt

1317d1c

brewmaster012 marked this pull request as ready for review October 8, 2024 03:40

brewmaster012 requested review from lumtis, fbac, ws4charlie and skosito as code owners October 8, 2024 03:40

lumtis reviewed Oct 8, 2024

View reviewed changes

lumtis mentioned this pull request Oct 8, 2024

add: whitelist SPL token to avoid erroneous user deposits #42

Closed

lumtis reviewed Oct 8, 2024

View reviewed changes

skosito reviewed Oct 8, 2024

View reviewed changes

use whitelist/unwhitelist across the board

e905e67

brewmaster012 requested review from lumtis and skosito October 8, 2024 14:54

Base automatically changed from enhance-pda-validation to main October 8, 2024 16:06

ws4charlie reviewed Oct 8, 2024

View reviewed changes

ws4charlie approved these changes Oct 8, 2024

View reviewed changes

merge main

75ac812

brewmaster012 changed the title ~~add: whitelist and de-whitelist SPL tokens~~ feat: whitelist and de-whitelist SPL tokens Oct 8, 2024

brewmaster012 changed the title ~~feat: whitelist and de-whitelist SPL tokens~~ feat: whitelist and unwhitelist SPL tokens Oct 8, 2024

skosito reviewed Oct 8, 2024

View reviewed changes

revert README due to merge issue

4e2688c

add PDA address back to README

7388e00

skosito approved these changes Oct 8, 2024

View reviewed changes

brewmaster012 merged commit 705831d into main Oct 8, 2024
14 checks passed

brewmaster012 deleted the whitelist-spl branch October 8, 2024 22:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: whitelist and unwhitelist SPL tokens #41

feat: whitelist and unwhitelist SPL tokens #41

brewmaster012 commented Oct 8, 2024 •

edited

Loading

codecov-commenter commented Oct 8, 2024 •

edited

Loading

lumtis Oct 8, 2024

skosito Oct 8, 2024

lumtis Oct 8, 2024

brewmaster012 Oct 8, 2024

lumtis Oct 8, 2024

brewmaster012 Oct 8, 2024

skosito Oct 8, 2024

lumtis Oct 8, 2024

skosito Oct 8, 2024

brewmaster012 Oct 8, 2024

skosito Oct 8, 2024

brewmaster012 Oct 8, 2024 •

edited

Loading

ws4charlie Oct 8, 2024

ws4charlie Oct 8, 2024

brewmaster012 Oct 8, 2024

ws4charlie left a comment

skosito Oct 8, 2024

brewmaster012 Oct 8, 2024

skosito Oct 8, 2024

brewmaster012 commented Oct 8, 2024

		@@ -19,6 +19,56 @@ const keyPair = ec.keyFromPrivate('5b81cdf52ba0766983acf8dd0072904733d92afe4dd34

		const usdcDecimals = 6;

		async function mintSPLToken(conn: anchor.web3.Connection, wallet: anchor.web3.Keypair, mint: anchor.web3.Keypair) {

		@@ -1,6 +1,7 @@

		Note: Mainnet-beta, testnet, devnet gateway program address:

		Mainnet-beta, testnet, devnet gateway program address:

feat: whitelist and unwhitelist SPL tokens #41

feat: whitelist and unwhitelist SPL tokens #41

Conversation

brewmaster012 commented Oct 8, 2024 • edited Loading

query whitelisted status

testing

codecov-commenter commented Oct 8, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brewmaster012 Oct 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ws4charlie left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brewmaster012 commented Oct 8, 2024

brewmaster012 commented Oct 8, 2024 •

edited

Loading

codecov-commenter commented Oct 8, 2024 •

edited

Loading

brewmaster012 Oct 8, 2024 •

edited

Loading