Implement more constructor dispatch in cfg (#1039)

This makes the following changes:
 - The data account for the solidity contract to be executed must always be the first to be passed
 - Solana deployment code is generated in CFG and not in emit.
 - One test fails without this fix: 

solang now requires data account to the first account solana-labs/solana-solidity.js#42
Once this is merged I'll release a new version of @solana/solidity npm library and re-enable the test

integration/solana/create_contract.spec.ts

@@ -15,7 +15,10 @@ describe('ChildContract', function () {
         ({ contract, storage } = await loadContract('creator', 'creator.abi'));
     });
 
-    it('Creates child contract', async function () {
+    // FIXME:
+    // Test disabled in https://github.com/hyperledger/solang/pull/1039
+    // shoulde be fixed by https://github.com/solana-labs/solana-solidity.js/pull/42
+    xit('Creates child contract', async function () {
         childPDA = await createProgramDerivedAddress(contract.program);
 
         const { logs } = await contract.functions.create_child({

integration/solana/simple.spec.ts

@@ -337,7 +337,7 @@ describe('Deploy solang contract and test', function () {
     });
 
     it('account storage too small dynamic alloc', async function () {
-        const { contract } = await loadContract('store', 'store.abi', [], 200);
+        const { contract } = await loadContract('store', 'store.abi', [], 233);
 
         // storage.sol needs 168 bytes on constructor, more for string data
 
@@ -348,7 +348,7 @@ describe('Deploy solang contract and test', function () {
     });
 
     it('account storage too small dynamic realloc', async function () {
-        const { contract } = await loadContract('store', 'store.abi', [], 210);
+        const { contract } = await loadContract('store', 'store.abi', [], 233);
 
         async function push_until_bang() {
             for (let i = 0; i < 100; i++) {

src/codegen/cfg.rs

@@ -197,6 +197,7 @@ pub enum ReturnCode {
     FunctionSelectorInvalid,
     AbiEncodingInvalid,
     InvalidDataError,
+    AccountDataTooSmall,
 }
 
 impl Instr {

src/codegen/dispatch.rs

@@ -1,11 +1,16 @@
 // SPDX-License-Identifier: Apache-2.0
 
-use crate::codegen::cfg::{ASTFunction, ControlFlowGraph, Instr, InternalCallTy, ReturnCode};
-use crate::codegen::vartable::Vartable;
-use crate::codegen::{Builtin, Expression};
-use crate::sema::ast::{Namespace, Parameter, RetrieveType, Type};
+use crate::codegen::{
+    cfg::{ASTFunction, ControlFlowGraph, Instr, InternalCallTy, ReturnCode},
+    vartable::Vartable,
+    Builtin, Expression,
+};
+use crate::{
+    sema::ast::{ArrayLength, Namespace, Parameter, RetrieveType, StructType, Type},
+    Target,
+};
 use num_bigint::{BigInt, Sign};
-use num_traits::Zero;
+use num_traits::{ToPrimitive, Zero};
 use solang_parser::pt;
 use solang_parser::pt::{FunctionTy, Loc};
 use std::sync::Arc;
@@ -273,9 +278,10 @@ fn add_dispatch_case(
     ));
 }
 
-/// Create the dispatch for a contract constructor. This case needs creates a new function in
-/// the CFG because we want to use de abi decoding implementation from codegen.
+/// Create the dispatch for a contract constructor. This case creates a new function in
+/// the CFG because we want to use the abi decoding implementation from codegen.
 pub(super) fn constructor_dispatch(
+    contract_no: usize,
     constructor_cfg_no: usize,
     all_cfg: &[ControlFlowGraph],
     ns: &mut Namespace,
@@ -335,6 +341,23 @@ pub(super) fn constructor_dispatch(
         );
     }
 
+    if ns.target == Target::Solana {
+        solana_deploy(contract_no, &mut vartab, &mut cfg, ns);
+    }
+
+    // Call storage initializer
+    cfg.add(
+        &mut vartab,
+        Instr::Call {
+            res: vec![],
+            return_tys: vec![],
+            call: InternalCallTy::Static {
+                cfg_no: ns.contracts[contract_no].initializer.unwrap(),
+            },
+            args: vec![],
+        },
+    );
+
     cfg.add(
         &mut vartab,
         Instr::Call {
@@ -358,3 +381,115 @@ pub(super) fn constructor_dispatch(
 
     cfg
 }
+
+/// On Solana, prepare the data account after deploy; ensure the account is
+/// large enough and write magic to it to show the account has been deployed.
+fn solana_deploy(
+    contract_no: usize,
+    vartab: &mut Vartable,
+    cfg: &mut ControlFlowGraph,
+    ns: &mut Namespace,
+) {
+    // Make sure that the data account is large enough. Read the size of the
+    // account via `tx.accounts[0].data.length`.
+    let account_length = Expression::Builtin(
+        Loc::Codegen,
+        vec![Type::Uint(32)],
+        Builtin::ArrayLength,
+        vec![Expression::StructMember(
+            Loc::Codegen,
+            Type::DynamicBytes,
+            Expression::Subscript(
+                Loc::Codegen,
+                Type::Struct(StructType::AccountInfo),
+                Type::Array(
+                    Type::Struct(StructType::AccountInfo).into(),
+                    vec![ArrayLength::Dynamic],
+                ),
+                Expression::Builtin(
+                    Loc::Codegen,
+                    vec![Type::Array(
+                        Type::Struct(StructType::AccountInfo).into(),
+                        vec![ArrayLength::Dynamic],
+                    )],
+                    Builtin::Accounts,
+                    vec![],
+                )
+                .into(),
+                Expression::NumberLiteral(Loc::Codegen, Type::Uint(32), BigInt::zero()).into(),
+            )
+            .into(),
+            2,
+        )],
+    );
+
+    let is_enough = Expression::MoreEqual(
+        Loc::Codegen,
+        Box::new(account_length),
+        Box::new(Expression::NumberLiteral(
+            Loc::Codegen,
+            Type::Uint(32),
+            ns.contracts[contract_no].fixed_layout_size.clone(),
+        )),
+    );
+
+    let enough = cfg.new_basic_block("enough".into());
+    let not_enough = cfg.new_basic_block("not_enough".into());
+
+    cfg.add(
+        vartab,
+        Instr::BranchCond {
+            cond: is_enough,
+            true_block: enough,
+            false_block: not_enough,
+        },
+    );
+
+    cfg.set_basic_block(not_enough);
+
+    cfg.add(
+        vartab,
+        Instr::ReturnCode {
+            code: ReturnCode::AccountDataTooSmall,
+        },
+    );
+
+    cfg.set_basic_block(enough);
+
+    // Write contract magic number to offset 0
+    cfg.add(
+        vartab,
+        Instr::SetStorage {
+            ty: Type::Uint(32),
+            value: Expression::NumberLiteral(
+                pt::Loc::Codegen,
+                Type::Uint(64),
+                BigInt::from(ns.contracts[contract_no].selector()),
+            ),
+            storage: Expression::NumberLiteral(pt::Loc::Codegen, Type::Uint(64), BigInt::zero()),
+        },
+    );
+
+    // Calculate heap offset
+    let fixed_fields_size = ns.contracts[contract_no]
+        .fixed_layout_size
+        .to_u64()
+        .unwrap();
+
+    // align on 8 byte boundary (round up to nearest multiple of 8)
+    let heap_offset = (fixed_fields_size + 7) & !7;
+
+    // Write heap offset to 12
+    cfg.add(
+        vartab,
+        Instr::SetStorage {
+            ty: Type::Uint(32),
+            value: Expression::NumberLiteral(
+                pt::Loc::Codegen,
+                Type::Uint(64),
+                BigInt::from(heap_offset),
+            ),
+            storage: Expression::NumberLiteral(pt::Loc::Codegen, Type::Uint(64), BigInt::from(12)),
+        },
+    );
+}

src/codegen/mod.rs

@@ -249,7 +249,7 @@ fn contract(contract_no: usize, ns: &mut Namespace, opt: &Options) {
 
             for cfg_no in 0..all_cfg.len() {
                 if all_cfg[cfg_no].ty == FunctionTy::Constructor && all_cfg[cfg_no].public {
-                    let dispatch_cfg = constructor_dispatch(cfg_no, &all_cfg, ns);
+                    let dispatch_cfg = constructor_dispatch(contract_no, cfg_no, &all_cfg, ns);
                     ns.contracts[contract_no].constructor_dispatch = Some(all_cfg.len());
                     all_cfg.push(dispatch_cfg);
                     break;

src/emit/solana/mod.rs

@@ -17,7 +17,7 @@ use inkwell::{AddressSpace, IntPredicate, OptimizationLevel};
 use num_traits::ToPrimitive;
 
 use crate::emit::ethabiencoder;
-use crate::emit::functions::{emit_functions, emit_initializer};
+use crate::emit::functions::emit_functions;
 use crate::emit::loop_builder::LoopBuilder;
 use crate::emit::{Binary, TargetRuntime};
 
@@ -29,7 +29,6 @@ pub struct SolanaTarget {
 pub struct Contract<'a> {
     magic: u32,
     contract: &'a ast::Contract,
-    storage_initializer: FunctionValue<'a>,
     constructor: Option<FunctionValue<'a>>,
     functions: HashMap<usize, FunctionValue<'a>>,
 }
@@ -78,13 +77,15 @@ impl SolanaTarget {
             ReturnCode::InvalidDataError,
             context.i32_type().const_int(2, false),
         );
+        binary.return_values.insert(
+            ReturnCode::AccountDataTooSmall,
+            context.i64_type().const_int(5u64 << 32, false),
+        );
         // externals
         target.declare_externals(&mut binary, ns);
 
         emit_functions(&mut target, &mut binary, contract, ns);
 
-        let storage_initializer = emit_initializer(&mut target, &mut binary, contract, ns);
-
         let constructor = contract
             .constructor_dispatch
             .map(|cfg_no| binary.functions[&cfg_no]);
@@ -98,7 +99,6 @@ impl SolanaTarget {
             &[Contract {
                 magic: target.magic,
                 contract,
-                storage_initializer,
                 constructor,
                 functions,
             }],
@@ -147,6 +147,10 @@ impl SolanaTarget {
             ReturnCode::AbiEncodingInvalid,
             context.i64_type().const_int(2u64 << 32, false),
         );
+        binary.return_values.insert(
+            ReturnCode::AccountDataTooSmall,
+            context.i64_type().const_int(5u64 << 32, false),
+        );
 
         // externals
         target.declare_externals(&mut binary, namespaces[0]);
@@ -165,8 +169,6 @@ impl SolanaTarget {
 
                 emit_functions(&mut target, &mut binary, contract, ns);
 
-                let storage_initializer = emit_initializer(&mut target, &mut binary, contract, ns);
-
                 let constructor = contract
                     .constructor_dispatch
                     .map(|cfg_no| binary.functions[&cfg_no]);
@@ -178,7 +180,6 @@ impl SolanaTarget {
                 contracts.push(Contract {
                     magic: target.magic,
                     contract,
-                    storage_initializer,
                     constructor,
                     functions,
                 });
@@ -428,41 +429,6 @@ impl SolanaTarget {
             .into_pointer_value()
     }
 
-    /// Returns the account data length of the executing binary
-    fn contract_storage_datalen<'b>(&self, binary: &Binary<'b>) -> IntValue<'b> {
-        let parameters = self.sol_parameters(binary);
-
-        let ka_cur = binary
-            .builder
-            .build_load(
-                binary
-                    .builder
-                    .build_struct_gep(parameters, 2, "ka_cur")
-                    .unwrap(),
-                "ka_cur",
-            )
-            .into_int_value();
-
-        binary
-            .builder
-            .build_load(
-                unsafe {
-                    binary.builder.build_gep(
-                        parameters,
-                        &[
-                            binary.context.i32_type().const_int(0, false),
-                            binary.context.i32_type().const_int(0, false),
-                            ka_cur,
-                            binary.context.i32_type().const_int(2, false),
-                        ],
-                        "data_len",
-                    )
-                },
-                "data_len",
-            )
-            .into_int_value()
-    }
-
     fn emit_dispatch<'b>(&mut self, binary: &mut Binary<'b>, contracts: &[Contract<'b>]) {
         let function = binary.module.get_function("solang_dispatch").unwrap();
 
@@ -573,8 +539,6 @@ impl SolanaTarget {
 
         binary.builder.position_at_end(constructor_block);
 
-        let contract_data_len = self.contract_storage_datalen(binary);
-
         for contract in contracts {
             let constructor_block = binary
                 .context
@@ -590,72 +554,6 @@ impl SolanaTarget {
                 constructor_block,
             ));
 
-            // do we have enough binary data
-            let fixed_fields_size = contract.contract.fixed_layout_size.to_u64().unwrap();
-
-            let is_enough = binary.builder.build_int_compare(
-                IntPredicate::UGE,
-                contract_data_len,
-                binary
-                    .context
-                    .i64_type()
-                    .const_int(fixed_fields_size, false),
-                "is_enough",
-            );
-
-            let not_enough = binary.context.append_basic_block(function, "not_enough");
-            let enough = binary.context.append_basic_block(function, "enough");
-
-            binary
-                .builder
-                .build_conditional_branch(is_enough, enough, not_enough);
-
-            binary.builder.position_at_end(not_enough);
-
-            binary.builder.build_return(Some(
-                &binary.context.i64_type().const_int(5u64 << 32, false),
-            ));
-
-            binary.builder.position_at_end(enough);
-
-            // write our magic value to the binary
-            binary.builder.build_store(
-                magic_value_ptr,
-                binary
-                    .context
-                    .i32_type()
-                    .const_int(contract.magic as u64, false),
-            );
-
-            // write heap_offset.
-            let heap_offset_ptr = unsafe {
-                binary.builder.build_gep(
-                    magic_value_ptr,
-                    &[binary.context.i64_type().const_int(3, false)],
-                    "heap_offset",
-                )
-            };
-
-            // align heap to 8 bytes
-            let heap_offset = (fixed_fields_size + 7) & !7;
-
-            binary.builder.build_store(
-                heap_offset_ptr,
-                binary.context.i32_type().const_int(heap_offset, false),
-            );
-
-            let arg_ty =
-                contract.storage_initializer.get_type().get_param_types()[0].into_pointer_type();
-
-            binary.builder.build_call(
-                contract.storage_initializer,
-                &[binary
-                    .builder
-                    .build_pointer_cast(sol_params, arg_ty, "")
-                    .into()],
-                "",
-            );
-
             // There is only one possible constructor
             let ret = if let Some(constructor_function) = contract.constructor {
                 binary

stdlib/solana.c

@@ -37,7 +37,7 @@ entrypoint(const uint8_t *input)
     {
         const SolAccountInfo *acc = &params.ka[account_no];
 
-        if (SolPubkey_same(params.account_id, acc->key))
+        if (SolPubkey_same(params.account_id, acc->key) && params.ka_cur == UINT64_MAX)
         {
             params.ka_cur = account_no;
         }
@@ -51,7 +51,7 @@ entrypoint(const uint8_t *input)
         }
     }
 
-    if (params.ka_cur == UINT64_MAX)
+    if (params.ka_cur != 0)
     {
         return ERROR_INVALID_INSTRUCTION_DATA;
     }
@@ -82,19 +82,28 @@ uint64_t external_call(uint8_t *input, uint32_t input_len, SolParameters *params
         .data_len = input_len,
     };
 
+    int meta_no = 1;
+
     for (int account_no = 0; account_no < params->ka_num; account_no++)
     {
         const SolAccountInfo *acc = &params->ka[account_no];
 
         if (SolPubkey_same(dest, acc->key))
         {
+            metas[0].pubkey = acc->key;
+            metas[0].is_writable = acc->is_writable;
+            metas[0].is_signer = acc->is_signer;
             instruction.program_id = acc->owner;
             params->ka_last_called = acc;
         }
+        else
+        {
 
-        metas[account_no].pubkey = acc->key;
-        metas[account_no].is_writable = acc->is_writable;
-        metas[account_no].is_signer = acc->is_signer;
+            metas[meta_no].pubkey = acc->key;
+            metas[meta_no].is_writable = acc->is_writable;
+            metas[meta_no].is_signer = acc->is_signer;
+            meta_no += 1;
+        }
     }
 
     if (instruction.program_id)
@@ -118,7 +127,7 @@ uint64_t create_contract(uint8_t *input, uint32_t input_len, uint64_t space, Sol
     // find a suitable new account and seed
     for (int i = 0; i < params->seeds_len; i++)
     {
-        SolAccountInfo *acc = &params->ka[i];
+        SolAccountInfo *acc = &params->ka[i + 1];
 
         if (acc->data_len == 0 && SolPubkey_same(&system_address, acc->owner))
         {
@@ -205,14 +214,23 @@ uint64_t create_contract(uint8_t *input, uint32_t input_len, uint64_t space, Sol
         .data_len = input_len,
     };
 
-    // A fresh account must be provided by the caller; find it
+    metas[0].pubkey = new_acc->key;
+    metas[0].is_writable = true;
+    metas[0].is_signer = false;
+
+    int meta_no = 1;
+
     for (int account_no = 0; account_no < params->ka_num; account_no++)
     {
         const SolAccountInfo *acc = &params->ka[account_no];
 
-        metas[account_no].pubkey = acc->key;
-        metas[account_no].is_writable = acc->is_writable;
-        metas[account_no].is_signer = acc->is_signer;
+        if (!SolPubkey_same(new_acc->key, acc->key))
+        {
+            metas[meta_no].pubkey = acc->key;
+            metas[meta_no].is_writable = acc->is_writable;
+            metas[meta_no].is_signer = acc->is_signer;
+            meta_no += 1;
+        }
     }
 
     params->ka_last_called = new_acc;

tests/solana.rs

@@ -303,7 +303,17 @@ fn serialize_parameters(
     v.write_u64::<LittleEndian>(vm.account_data.len() as u64)
         .unwrap();
 
-    // first do the seeds
+    // first do the account data
+    let data_account = &vm.stack[0].data;
+
+    serialize_account(
+        &mut v,
+        &mut refs,
+        data_account,
+        &vm.account_data[data_account],
+    );
+
+    // second do the seeds
     for (acc, _) in seeds {
         let data = &vm.account_data[acc];
 
@@ -312,9 +322,9 @@ fn serialize_parameters(
         serialize_account(&mut v, &mut refs, acc, data);
     }
 
+    // then the rest of the accounts
     for (acc, data) in &vm.account_data {
-        //println!("acc:{} {}", hex::encode(acc), hex::encode(&data.0));
-        if !seeds.iter().any(|seed| seed.0 == *acc) {
+        if !seeds.iter().any(|seed| seed.0 == *acc) && acc != data_account {
             serialize_account(&mut v, &mut refs, acc, data);
         }
     }
@@ -1414,6 +1424,8 @@ impl<'a> SyscallObject<UserError> for SyscallInvokeSignedC<'a> {
                     hex::encode(instruction.program_id.0)
                 );
 
+                assert_eq!(data_id, instruction.accounts[0].pubkey.0);
+
                 let p = vm
                     .programs
                     .iter()
@@ -1570,8 +1582,11 @@ impl VirtualMachine {
     }
 
     fn constructor(&mut self, name: &str, args: &[Token]) {
-        let program = &self.stack[0];
+        self.constructor_expected(0, name, args)
+    }
 
+    fn constructor_expected(&mut self, expected: u64, name: &str, args: &[Token]) {
+        let program = &self.stack[0];
         println!("constructor for {}", hex::encode(program.data));
 
         let mut calldata = VirtualMachine::input(&program.data, &self.origin, name, &[]);
@@ -1581,8 +1596,9 @@ impl VirtualMachine {
         };
 
         let res = self.execute(&calldata, &[]);
+
         println!("res:{:?}", res);
-        assert!(matches!(res, Ok(0)));
+        assert_eq!(res, Ok(expected));
     }
 
     fn function(
@@ -1749,60 +1765,65 @@ impl VirtualMachine {
 
     fn validate_account_data_heap(&self) -> usize {
         let data = &self.account_data[&self.stack[0].data].data;
-        let mut count = 0;
 
-        let mut prev_offset = 0;
-        let return_len = LittleEndian::read_u32(&data[4..]) as usize;
-        let return_offset = LittleEndian::read_u32(&data[8..]) as usize;
-        let mut offset = LittleEndian::read_u32(&data[12..]) as usize;
+        if LittleEndian::read_u32(&data[0..]) != 0 {
+            let mut count = 0;
 
-        // The return_offset/len fields are no longer used (we should remove them at some point)
-        assert_eq!(return_len, 0);
-        assert_eq!(return_offset, 0);
+            let mut prev_offset = 0;
+            let return_len = LittleEndian::read_u32(&data[4..]) as usize;
+            let return_offset = LittleEndian::read_u32(&data[8..]) as usize;
+            let mut offset = LittleEndian::read_u32(&data[12..]) as usize;
 
-        println!(
-            "static: length:{:x} {}",
-            offset - 16,
-            hex::encode(&data[16..offset])
-        );
-
-        loop {
-            let next = LittleEndian::read_u32(&data[offset..]) as usize;
-            let prev = LittleEndian::read_u32(&data[offset + 4..]) as usize;
-            let length = LittleEndian::read_u32(&data[offset + 8..]) as usize;
-            let allocate = LittleEndian::read_u32(&data[offset + 12..]) as usize;
-
-            if allocate == 1 {
-                count += 1;
-            }
+            // The return_offset/len fields are no longer used (we should remove them at some point)
+            assert_eq!(return_len, 0);
+            assert_eq!(return_offset, 0);
 
             println!(
-                "offset:{:x} prev:{:x} next:{:x} length:{} allocated:{} {}",
-                offset + 16,
-                prev + 16,
-                next + 16,
-                length,
-                allocate,
-                hex::encode(&data[offset + 16..offset + 16 + length])
+                "static: length:{:x} {}",
+                offset - 16,
+                hex::encode(&data[16..offset])
             );
 
-            assert_eq!(prev, prev_offset);
-            prev_offset = offset;
+            loop {
+                let next = LittleEndian::read_u32(&data[offset..]) as usize;
+                let prev = LittleEndian::read_u32(&data[offset + 4..]) as usize;
+                let length = LittleEndian::read_u32(&data[offset + 8..]) as usize;
+                let allocate = LittleEndian::read_u32(&data[offset + 12..]) as usize;
 
-            if next == 0 {
-                assert_eq!(length, 0);
-                assert_eq!(allocate, 0);
+                if allocate == 1 {
+                    count += 1;
+                }
 
-                break;
-            }
+                println!(
+                    "offset:{:x} prev:{:x} next:{:x} length:{} allocated:{} {}",
+                    offset + 16,
+                    prev + 16,
+                    next + 16,
+                    length,
+                    allocate,
+                    hex::encode(&data[offset + 16..offset + 16 + length])
+                );
 
-            let space = next - offset - 16;
-            assert!(length <= space);
+                assert_eq!(prev, prev_offset);
+                prev_offset = offset;
 
-            offset = next;
-        }
+                if next == 0 {
+                    assert_eq!(length, 0);
+                    assert_eq!(allocate, 0);
+
+                    break;
+                }
 
-        count
+                let space = next - offset - 16;
+                assert!(length <= space);
+
+                offset = next;
+            }
+
+            count
+        } else {
+            0
+        }
     }
 
     pub fn events(&self) -> Vec<RawLog> {

tests/solana_tests/create_contract.rs

@@ -123,3 +123,19 @@ fn two_contracts() {
 
     vm.logs.truncate(0);
 }
+
+#[test]
+fn account_too_small() {
+    let mut vm = build_solidity(
+        r#"
+        contract bar {
+            int[200] foo1;
+        }"#,
+    );
+
+    let data = vm.stack[0].data;
+
+    vm.account_data.get_mut(&data).unwrap().data.truncate(100);
+
+    vm.constructor_expected(5 << 32, "bar", &[]);
+}