Allow seeds to be passed into functions as string[] or string[][] #1547

seanyoung · 2023-09-21T08:40:36Z

This fixes this Solidity and similar constructs.

import "solana-library/spl_token.sol";

contract c {

    // Invoke the token program to mint tokens to a token account, using a PDA as the mint authority
    function mintTo(address mint, address account, address authority, uint64 amount, bytes[] seeds) internal {
        // Prepare instruction data
        bytes instructionData = new bytes(9);
        instructionData[0] = uint8(7); // MintTo instruction index
        instructionData.writeUint64LE(amount, 1); // Amount to mint

        // Prepare accounts required by instruction
        AccountMeta[3] metas = [
            AccountMeta({pubkey: mint, is_writable: true, is_signer: false}),
            AccountMeta({pubkey: account, is_writable: true, is_signer: false}),
            AccountMeta({pubkey: authority, is_writable: true, is_signer: true})
        ];

        // Invoke the token program with prepared accounts and instruction data
        SplToken.tokenProgramId.call{accounts: metas, seeds: seeds}(instructionData);
    }
}

Fixes #1433

src/emit/expression.rs

src/emit/instructions.rs

src/sema/expression/literals.rs

tests/contract_testcases/solana/bad_seeds_on_external_call.sol

LucasSte · 2023-09-21T15:17:37Z

import {AccountMeta} from 'solana';
            contract pda {
                address constant tokenProgramId = address"TokenkegQfeZyiNwAJbNbGKPFXCWuBvf9Ss623VQ5DA";
                address constant SYSVAR_RENT_PUBKEY = address"SysvarRent111111111111111111111111111111111";
                function test_bytes4(string[][] seeds) public {
                    bytes instr = new bytes(1);
                    instr[0] = 0x95;
                    AccountMeta[1] metas = [
                        AccountMeta({pubkey: SYSVAR_RENT_PUBKEY, is_writable: false, is_signer: false})
                    ];
                    tokenProgramId.call{seeds: seeds, accounts: metas}(instr);
                }
            }

Is this expected?

error: conversion from string[][] to bytes slice slice not possible
   ┌─ /Users/lucasste/Documents/solang/examples/test.sol:11:48
   │
11 │                     tokenProgramId.call{seeds: seeds, accounts: metas}(instr);
   │

LucasSte · 2023-09-21T15:23:04Z

            import {AccountMeta} from 'solana';
            contract pda {
                address constant tokenProgramId = address"TokenkegQfeZyiNwAJbNbGKPFXCWuBvf9Ss623VQ5DA";
                address constant SYSVAR_RENT_PUBKEY = address"SysvarRent111111111111111111111111111111111";
                function test(bytes[] dyn, address[] addr, bytes5[] b5) public {
                    bytes instr = new bytes(1);
                    instr[0] = 0x95;
                    AccountMeta[1] metas = [
                        AccountMeta({pubkey: SYSVAR_RENT_PUBKEY, is_writable: false, is_signer: false})
                    ];
                    bytes3 foo = "foo";
                    string foo2 = "123456";

                    tokenProgramId.call{seeds: [ dyn, addr, b5, [foo, foo2] ], accounts: metas}(instr);
                }
            }

Is this expected (part 2)?

error: conversion from string to bytes not possible
   ┌─ /Users/lucasste/Documents/solang/examples/test.sol:15:71
   │
15 │                     tokenProgramId.call{seeds: [ dyn, addr, b5, [foo, foo2] ], accounts: metas}(instr);
   │

LucasSte · 2023-09-21T15:24:35Z

Strings should be allowed, shouldn't they?

            import {AccountMeta} from 'solana';
            contract pda {
                address constant tokenProgramId = address"TokenkegQfeZyiNwAJbNbGKPFXCWuBvf9Ss623VQ5DA";
                address constant SYSVAR_RENT_PUBKEY = address"SysvarRent111111111111111111111111111111111";
                function test(bytes[] dyn, string[] addr, bytes5[] b5) public {
                    bytes instr = new bytes(1);
                    instr[0] = 0x95;
                    AccountMeta[1] metas = [
                        AccountMeta({pubkey: SYSVAR_RENT_PUBKEY, is_writable: false, is_signer: false})
                    ];
                    bytes3 foo = "foo";
                    bytes6 foo2 = "123456";

                    tokenProgramId.call{seeds: [ dyn, addr, b5, [foo, foo2] ], accounts: metas}(instr);
                }
            }

error: type string found where array bytes slice expected
   ┌─ /Users/lucasste/Documents/solang/examples/test.sol:14:55
   │
14 │                     tokenProgramId.call{seeds: [ dyn, addr, b5, [foo, foo2] ], accounts: metas}(instr);
   │

tests/solana_tests/call.rs

seanyoung · 2023-09-24T09:24:12Z

Type string is now allowed as a seed

LucasSte · 2023-09-25T18:03:31Z

src/emit/expression.rs

+            let dest = call!("__malloc", &[i32_const!(bytes_length).into()])
+                .try_as_basic_value()
+                .left()
+                .unwrap()
+                .into_pointer_value();


You've already allocated memory here to save the bytes value. It is necessary to allocate again?

You can swap endianness in place and return a pointer to the stack allocated memory.

[1, 2, 3, 4, 5] ^ ^ |-----------| ==> swap(1, 5) [5, 2, 3, 4, 1] ^ ^ |-----| ==> swap(2, 4) [5, 4, 3, 2, 1]

The other allocation is on the stack and that is a single allocation. This malloc allocation is per-slice.

You do not need to allocate space in the stack and in the heap. You can allocate the memory in one place (heap or stack), store the integer there, change the endianness in place and return the pointer to that memory region.

This might be something for another PR. We do all our endian-ness swaps like this.

In fact, I think bytesN should be represented by a byte array and then all these swaps would go away.

src/emit/polkadot/mod.rs

LucasSte · 2023-09-25T19:46:22Z

import {AccountMeta} from 'solana';
            contract pda {
                address constant tokenProgramId = address"TokenkegQfeZyiNwAJbNbGKPFXCWuBvf9Ss623VQ5DA";
                address constant SYSVAR_RENT_PUBKEY = address"SysvarRent111111111111111111111111111111111";
                function test(bytes[] dyn, address[] addr, bytes5[] b5, string f) public {
                    bytes instr = new bytes(1);
                    instr[0] = 0x95;
                    AccountMeta[1] metas = [
                        AccountMeta({pubkey: SYSVAR_RENT_PUBKEY, is_writable: false, is_signer: false})
                    ];
                    bytes3 foo = "foo";
                    tokenProgramId.call{seeds: [ dyn, addr, b5, [f] ], accounts: metas}(instr);
                }
            }

Is this correct?

thread 'main' panicked at 'internal error: entered unreachable code', src/emit/expression.rs:2123:14
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

LucasSte · 2023-09-25T19:49:36Z

I also tested the following example, but I am not confident it is going to be a common construct, so perhaps we should not worry about it.

import {AccountMeta} from 'solana';
            contract pda {
                address constant tokenProgramId = address"TokenkegQfeZyiNwAJbNbGKPFXCWuBvf9Ss623VQ5DA";
                address constant SYSVAR_RENT_PUBKEY = address"SysvarRent111111111111111111111111111111111";
                function test_bytes(bytes[][] seeds) public {
                    bytes instr = new bytes(1);
                    instr[0] = 0x95;
                    AccountMeta[1] metas = [
                        AccountMeta({pubkey: SYSVAR_RENT_PUBKEY, is_writable: false, is_signer: false})
                    ];
                    tokenProgramId.call{seeds: [seeds[0], seeds[1]], accounts: metas}(instr);
                }
            }

error: type bytes[] found where array expected
   ┌─ /Users/lucasste/Documents/solang/examples/test.sol:13:49
   │
13 │                     tokenProgramId.call{seeds: [seeds[0], seeds[1]], accounts: metas}(instr);
   │                                                 ^^^^^^^^

error: type bytes[] found where array expected
   ┌─ /Users/lucasste/Documents/solang/examples/test.sol:13:59
   │
13 │                     tokenProgramId.call{seeds: [seeds[0], seeds[1]], accounts: metas}(instr);
   │                                                           ^^^^^^^^

src/emit/binary.rs

src/emit/expression.rs

seanyoung · 2023-09-27T09:25:50Z

@LucasSte I've added the two cases you found as a test cases

xermicus

LGTM overall, just some cosmetics

xermicus · 2023-09-27T10:26:05Z

src/emit/expression.rs

@@ -2120,3 +2122,227 @@ fn runtime_cast<'a>(
        _ => unreachable!(),
    }
 }
+
+/// Emit expression into a slice


Nitpick: Superfluous. A more useful comment could for example say when this is needed or explain the return type (which I'd guess is a pointer to the slice and the length of the slice)?

You're right, that comment is terrible.

src/emit/expression.rs

xermicus

As discussed in the standup, it would be great to have those SAFETY comments, explaining our thoughts around why all those arguments to the LLVM GEP builder are correct. But it's not only in this place, I'll leave it up to you.

Fixes hyperledger-solang#1433 Signed-off-by: Sean Young <[email protected]>

Signed-off-by: Sean Young <[email protected]>

seanyoung · 2023-09-27T15:41:38Z

As discussed in the standup, it would be great to have those SAFETY comments, explaining our thoughts around why all those arguments to the LLVM GEP builder are correct. But it's not only in this place, I'll leave it up to you.

What do you think of the SAFETY comments I've just added?

xermicus · 2023-09-27T16:05:20Z

What do you think of the SAFETY comments I've just added?

Great, this is exactly what I meant, thanks!

LucasSte

I tested it locally and couldn't find any problem. Codewise, everything looks good. I have only one last question.

Should there be an integration test for this feature?

src/emit/expression.rs

Co-authored-by: Lucas Steuernagel <[email protected]> Signed-off-by: Sean Young <[email protected]>

Signed-off-by: Sean Young <[email protected]>

seanyoung · 2023-09-28T11:15:28Z

I tested it locally and couldn't find any problem. Codewise, everything looks good. I have only one last question.

Should there be an integration test for this feature?

I've added a simple test.

This will need re-writing for your branch.

There is also a fix for the "blockhash not found" issue (at least it fixes it locally)

seanyoung marked this pull request as ready for review September 21, 2023 13:23

seanyoung requested review from xermicus and LucasSte as code owners September 21, 2023 13:23

seanyoung added the solana The Solana target label Sep 21, 2023

LucasSte reviewed Sep 21, 2023

View reviewed changes

tests/solana_tests/call.rs Show resolved Hide resolved

seanyoung force-pushed the seeds branch from 5b9db53 to acecd39 Compare September 21, 2023 20:06

seanyoung requested a review from LucasSte September 24, 2023 09:24

LucasSte reviewed Sep 25, 2023

View reviewed changes

xermicus reviewed Sep 26, 2023

View reviewed changes

src/emit/binary.rs Outdated Show resolved Hide resolved

src/emit/expression.rs Outdated Show resolved Hide resolved

seanyoung force-pushed the seeds branch 2 times, most recently from c5e81da to 2958604 Compare September 27, 2023 08:46

seanyoung requested review from xermicus and LucasSte September 27, 2023 09:44

xermicus reviewed Sep 27, 2023

View reviewed changes

seanyoung requested a review from xermicus September 27, 2023 13:21

xermicus approved these changes Sep 27, 2023

View reviewed changes

seanyoung added 6 commits September 27, 2023 16:38

Allow seeds to be passed into functions as string[] or string[][]

c5d9d28

Fixes hyperledger-solang#1433 Signed-off-by: Sean Young <[email protected]>

minor improvements

858be98

Signed-off-by: Sean Young <[email protected]>

improve tests

f1f0569

Signed-off-by: Sean Young <[email protected]>

more test

82050e2

Signed-off-by: Sean Young <[email protected]>

Allow string type and add comment

9828851

Signed-off-by: Sean Young <[email protected]>

improve error message

c6e7d6f

Signed-off-by: Sean Young <[email protected]>

seanyoung added 10 commits September 27, 2023 16:38

make 1 fixed test work

cb06737

Signed-off-by: Sean Young <[email protected]>

more fixes

8d6746f

Signed-off-by: Sean Young <[email protected]>

another fix

016fe0c

Signed-off-by: Sean Young <[email protected]>

Move emit_context

34dcd35

Signed-off-by: Sean Young <[email protected]>

fixes hyperledger-solang#2

50c6a2f

Signed-off-by: Sean Young <[email protected]>

Review comments

c21a3b2

Signed-off-by: Sean Young <[email protected]>

Add lucas' test cases

233361d

Signed-off-by: Sean Young <[email protected]>

improve

ba08a70

Signed-off-by: Sean Young <[email protected]>

Improve comment

32ce29a

Signed-off-by: Sean Young <[email protected]>

add SAFETY comments

02bcfff

Signed-off-by: Sean Young <[email protected]>

seanyoung force-pushed the seeds branch from a89b0a7 to 02bcfff Compare September 27, 2023 15:38

LucasSte reviewed Sep 27, 2023

View reviewed changes

src/emit/expression.rs Outdated Show resolved Hide resolved

seanyoung and others added 2 commits September 28, 2023 08:19

Update src/emit/expression.rs

0e3ac8a

Co-authored-by: Lucas Steuernagel <[email protected]> Signed-off-by: Sean Young <[email protected]>

Add test cases

81d7bb2

Signed-off-by: Sean Young <[email protected]>

seanyoung requested a review from LucasSte September 28, 2023 11:15

LucasSte approved these changes Sep 28, 2023

View reviewed changes

seanyoung merged commit 5675a34 into hyperledger-solang:main Sep 28, 2023
16 checks passed

seanyoung deleted the seeds branch September 28, 2023 14:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow seeds to be passed into functions as string[] or string[][] #1547

Allow seeds to be passed into functions as string[] or string[][] #1547

seanyoung commented Sep 21, 2023 •

edited

Loading

LucasSte commented Sep 21, 2023

LucasSte commented Sep 21, 2023

LucasSte commented Sep 21, 2023

seanyoung commented Sep 24, 2023

LucasSte Sep 25, 2023

seanyoung Sep 26, 2023

LucasSte Sep 26, 2023 •

edited

Loading

seanyoung Sep 27, 2023

LucasSte commented Sep 25, 2023

LucasSte commented Sep 25, 2023

seanyoung commented Sep 27, 2023

xermicus left a comment

xermicus Sep 27, 2023

seanyoung Sep 27, 2023

xermicus left a comment •

edited

Loading

seanyoung commented Sep 27, 2023

xermicus commented Sep 27, 2023

LucasSte left a comment

seanyoung commented Sep 28, 2023

Allow seeds to be passed into functions as string[] or string[][] #1547

Allow seeds to be passed into functions as string[] or string[][] #1547

Conversation

seanyoung commented Sep 21, 2023 • edited Loading

LucasSte commented Sep 21, 2023

LucasSte commented Sep 21, 2023

LucasSte commented Sep 21, 2023

seanyoung commented Sep 24, 2023

LucasSte Sep 25, 2023

Choose a reason for hiding this comment

seanyoung Sep 26, 2023

Choose a reason for hiding this comment

LucasSte Sep 26, 2023 • edited Loading

Choose a reason for hiding this comment

seanyoung Sep 27, 2023

Choose a reason for hiding this comment

LucasSte commented Sep 25, 2023

LucasSte commented Sep 25, 2023

seanyoung commented Sep 27, 2023

xermicus left a comment

Choose a reason for hiding this comment

xermicus Sep 27, 2023

Choose a reason for hiding this comment

seanyoung Sep 27, 2023

Choose a reason for hiding this comment

xermicus left a comment • edited Loading

Choose a reason for hiding this comment

seanyoung commented Sep 27, 2023

xermicus commented Sep 27, 2023

LucasSte left a comment

Choose a reason for hiding this comment

seanyoung commented Sep 28, 2023

seanyoung commented Sep 21, 2023 •

edited

Loading

LucasSte Sep 26, 2023 •

edited

Loading

xermicus left a comment •

edited

Loading