Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

add: allow withdraw SOL to PDA #37

Merged
merged 13 commits into from
Oct 4, 2024
+3 −11
Merged

add: allow withdraw SOL to PDA #37

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
5abfbaf
remove memo from deposit_spl_token
brewmaster012 Oct 2, 2024
6f4eccd
refactor deposit*_and_call instructions to reuse deposit*
brewmaster012 Oct 2, 2024
acbd25c
format
brewmaster012 Oct 2, 2024
b0318bb
update comment in code that is inaccurate
brewmaster012 Oct 2, 2024
0e18af4
address some CI complaints
brewmaster012 Oct 2, 2024
f352dc3
add some light comments to instructions
brewmaster012 Oct 2, 2024
f9151ac
silence intentional unused variable warnings
brewmaster012 Oct 2, 2024
0d81e93
add some test
brewmaster012 Oct 3, 2024
d1497af
Merge branch 'main' into withdraw-to-pda
brewmaster012 Oct 3, 2024
605b695
allow withdraw to PDA (other than system account)
brewmaster012 Oct 3, 2024
30e11cd
add a test to withdraw SOL to a ATA address (PDA)
brewmaster012 Oct 3, 2024
7ca038b
Update tests/protocol-contracts-solana.ts
brewmaster012 Oct 4, 2024
0b0027e
substantiated comment regarding UncheckedAccount
brewmaster012 Oct 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion programs/protocol-contracts-solana/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -350,8 +350,9 @@ pub struct Withdraw<'info> {

#[account(mut)]
pub pda: Account<'info, Pda>,
/// CHECK: to account is not read so it's not dangerous
Copy link
Contributor

Choose a reason for hiding this comment

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

is there a way to expand this comment a bit? account is still marked as mutable, and writing to unchecked accounts (which doesnt check owner of account) seems more dangerous than reading

maybe we can add explanation why writing to it is not dangerous, but up to you if it makes sense

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i'll keep it simple in the comment, but expand here:

Account<'info, T> is an Anchor convenient struct that automatically does "ownership" check, which UncheckedAccount explicitly does not.

Ownership is usually critical because these accounts are completely supplied by user (tx builder) and
could be anything. The onwership validation makes sure the user supplied the correct account.
For example, in the above pub pda: Account<'info, Pda>, Anchor does ownership check
Account. info. owner == T::owner(), which checks whether the supplied PDA account is in fact
owned by the current program (i.e. created by the current program). This is crucial otherwise
the program would be directed to read any user supplied potential malicious PDA account.

For the to field, obviously we don't care who its owner is because we don't read it. Whatever
tx builder tells us to send to, we happily send to. In this case Anchor requires program to explicit
exempt ownership checking by the UncheckedAccount struct and also the comment ///CHECK.

#[account(mut)]
pub to: SystemAccount<'info>,
pub to: UncheckedAccount<'info>,
}

#[derive(Accounts)]
Expand Down
12 changes: 2 additions & 10 deletions tests/protocol-contracts-solana.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,15 +248,6 @@ describe("some tests", () => {

it("deposit and withdraw 0.5 SOL from Gateway with ECDSA signature", async () => {
await gatewayProgram.methods.deposit(new anchor.BN(1_000_000_000), Array.from(address)).accounts({pda: pdaAccount}).rpc();
// const transaction = new anchor.web3.Transaction();
// transaction.add(
// web3.SystemProgram.transfer({
// fromPubkey: wallet.publicKey,
// toPubkey: pdaAccount,
// lamports: 1_000_000_000,
// })
// );
// await anchor.web3.sendAndConfirmTransaction(conn, transaction, [wallet]);
let bal1 = await conn.getBalance(pdaAccount);
console.log("pda account balance", bal1);
expect(bal1).to.be.gte(1_000_000_000);
Expand All @@ -271,7 +262,8 @@ describe("some tests", () => {
// );
const nonce = pdaAccountData.nonce;
const amount = new anchor.BN(500000000);
const to = wallet.publicKey;
// const to = wallet.publicKey;
brewmaster012 marked this conversation as resolved.
Show resolved Hide resolved
const to = pda_ata.address;
const buffer = Buffer.concat([
Buffer.from("withdraw","utf-8"),
chain_id_bn.toArrayLike(Buffer, 'be', 8),
Expand Down
Loading