Add Soroban target foundations. #1602

salaheldinsoliman · 2023-11-26T19:01:34Z

This PR is a follow up to this PR: #1138 and #1129 , both of which discuss adding Soroban as a target for Solang.
The PR addresses three main points:
1- Soroban contracts have no dispatcher (a single point of entry). Therefore, externally callable functions in the contract should preserve their original name (if possible).
2- Soroban functions do not have their outputs as pointers in the inputs. Instead, they return the value at the end of execution.
3-Linking Soroban WASM contracts is pretty much similar to Polkadot's WASM, but with the following minor differences:
a- public functions are exported.
b- host functions (of course)

The next steps for this PR would be: (to be followed in another PRs)
1- Implement host function invocations. (The VM interface)
2- Implement XDR encoding and decoding.
3- Add Integration tests, and complete the mock VM implementation in soroban_tests.

LucasSte · 2023-11-27T14:56:10Z

src/codegen/cfg.rs

+
+impl Display for CFGName {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        // print all elemnts of the CFGName struct, separated by a ::, in this order: contract_name::function_type::function_name


Suggested change

// print all elemnts of the CFGName struct, separated by a ::, in this order: contract_name::function_type::function_name

// print all elements of the CFGName struct, separated by a ::, in this order: contract_name::function_type::function_name

LucasSte · 2023-11-27T15:00:07Z

src/emit/binary.rs

+            match returns.iter().next() {
+                Some(ret) => return self.llvm_type(ret, ns).fn_type(&args, false),
+                None => return self.context.void_type().fn_type(&args, false),


What happens if the function has multiple returns?

I think the compiled function could return the multiple return values inside a Soroban Vec. This would require adding support for the host functions which I see in the description are planned as a next thing to implement, so I don't think this could be addressed until after that work was done.

LucasSte · 2023-11-27T15:02:32Z

src/emit/soroban/mod.rs

+            let name;
+            let linkage;
+            if cfg.public {
+                linkage = Linkage::External;
+
+                name = match cfg.function_no {
+                    ASTFunction::SolidityFunction(no) => {
+                        if dups.contains(&no) {
+                            cfg.name.llvm_symbol.clone()
+                        } else {
+                            cfg.original_function_name(ns)
+                        }
+                    }
+                    _ => cfg.name.llvm_symbol.clone(),
+                };
+
+                Self::emit_function_spec_entry(context, cfg, name.clone(), binary);
+            } else {
+                linkage = Linkage::Internal;
+                name = cfg.name();
+            };


This can be written as:

let (name, linkage) = if cfg.public { ... } else {...}

xermicus · 2023-11-28T08:37:43Z

Cargo.toml

@@ -71,6 +71,7 @@ forge-fmt = { version = "0.2.0", optional = true }
 # We don't use ethers-core directly, but need the correct version for the
 # build to work.
 ethers-core = { version = "2.0.10", optional = true }
+soroban-sdk = {version = "20.0.0-rc2", features = ["testutils"]}


Suggested change

soroban-sdk = {version = "20.0.0-rc2", features = ["testutils"]}

soroban-sdk = { version = "20.0.0-rc2", features = ["testutils"] }

This adds more than 100 dependencies. Can we start introducing per target feature flags to gate it (can go into default features)? Since this target is completely fresh it would be a great opportunity.

codecov · 2023-11-28T13:20:00Z

Codecov Report

Attention: 274 lines in your changes are missing coverage. Please review.

Comparison is base (a2ebcd1) 88.00% compared to head (9cbf17d) 87.67%.

Files	Patch %	Lines
src/emit/soroban/target.rs	0.00%	266 Missing ⚠️
src/emit/soroban/mod.rs	98.60%	2 Missing ⚠️
src/bin/cli/mod.rs	0.00%	1 Missing ⚠️
src/codegen/events/mod.rs	0.00%	1 Missing ⚠️
src/emit/binary.rs	87.50%	1 Missing ⚠️
src/lib.rs	66.66%	1 Missing ⚠️
src/linker/mod.rs	80.00%	1 Missing ⚠️
src/sema/yul/builtin.rs	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1602      +/-   ##
==========================================
- Coverage   88.00%   87.67%   -0.33%     
==========================================
  Files         133      136       +3     
  Lines       64970    65452     +482     
==========================================
+ Hits        57176    57385     +209     
- Misses       7794     8067     +273

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

src/codegen/cfg.rs

src/emit/soroban/mod.rs

src/sema/ast.rs

src/codegen/yul/mod.rs

seanyoung · 2023-11-29T11:16:19Z

@salaheldinsoliman polkadot already has a mechanism for dealing with duplicate function names, see mangled_name_contracts. It feels like you're re-inventing this again, and now we have two mechanisms for this in solang - not pretty

xermicus · 2023-11-29T12:09:52Z

Yep, and it's easy to not get it completely right throughout the codebase and overlook something wrt. mangled function names. I agree @seanyoung

Signed-off-by: salaheldinsoliman <salaheldin_sameh@aucegypt.edu>

salaheldinsoliman · 2023-12-01T13:30:43Z

@seanyoung thanks for the pointer, I totally overlooked mangled_name and mangled_name_contracts

Signed-off-by: salaheldinsoliman <salaheldin_sameh@aucegypt.edu>

seanyoung

Look good! I don't think anything can be done about the missing code coverage.

seanyoung · 2023-12-02T15:20:20Z

@salaheldinsoliman would you mind adding something to the documentation and CHANGELOG.md please 🙏

Signed-off-by: salaheldinsoliman <salaheldin_sameh@aucegypt.edu>

salaheldinsoliman · 2023-12-03T20:33:39Z

@salaheldinsoliman would you mind adding something to the documentation and CHANGELOG.md please 🙏

@seanyoung I added the change to CHANGELOG.md, but I think mentioning the Soroban target in the documentation should wait until the encoding/decoding and the VM interface are implemented.

xermicus · 2023-12-05T11:35:25Z

Cargo.toml

@@ -71,6 +71,7 @@ forge-fmt = { version = "0.2.0", optional = true }
 # We don't use ethers-core directly, but need the correct version for the
 # build to work.
 ethers-core = { version = "2.0.10", optional = true }
+soroban-sdk = { version = "20.0.0-rc2", features = ["testutils"] }


Again,can be gated behind a soroban feature flag. We already know we need to add per-target feature flags anyways. I'd appreciate it.

I mean this is so far only used in a single module in emit and in tests, making this trivial to gate for now

xermicus · 2023-12-06T08:52:52Z

Cargo.toml

@@ -71,6 +71,7 @@ forge-fmt = { version = "0.2.0", optional = true }
 # We don't use ethers-core directly, but need the correct version for the
 # build to work.
 ethers-core = { version = "2.0.10", optional = true }
+soroban-sdk = { version = "20.0.0-rc2", features = ["testutils"], optional = true }


Is testutils maybe only used in tests right?

yes, and primarily it allows compiling contracts natively -- as rust code that registers itself as a trait object with the soroban host, rather than as a chunk of wasm VM code.

xermicus · 2023-12-06T09:00:57Z

src/codegen/dispatch/mod.rs

@@ -17,5 +17,7 @@ pub(super) fn function_dispatch(
        Target::Polkadot { .. } | Target::EVM => {
            polkadot::function_dispatch(contract_no, all_cfg, ns, opt)
        }
+        #[cfg(feature = "soroban")]


I don't think we need to double down with the feature flags on target specific behavior at runtime (where that code doesn't depend on the soroban SDK dependency). The code base is not yet ready for this, doing this for all target will massively clutter it...

xermicus · 2023-12-06T09:04:24Z

src/emit/instructions.rs

-                let arg = function.get_nth_param((returns_offset + i) as u32).unwrap();
-                let retval = expression(target, bin, val, &w.vars, function, ns);
+        Instr::Return { value } => match ns.target {
+            #[cfg(feature = "soroban")]


Same here, this feature flag might be superfluous at the moment

xermicus · 2023-12-06T09:04:45Z

src/emit/binary.rs

@@ -758,6 +766,13 @@ impl<'a> Binary<'a> {
            .map(|ty| self.llvm_var_ty(ty, ns).into())
            .collect::<Vec<BasicMetadataTypeEnum>>();

+        #[cfg(feature = "soroban")]


Probably here too

xermicus · 2023-12-06T09:08:41Z

src/sema/namespace.rs

@@ -41,6 +41,8 @@ impl Namespace {
                value_length,
            } => (address_length, value_length),
            Target::Solana => (32, 8),
+            #[cfg(feature = "soroban")]


Again here.. If we want to go that route, it had to be consistent everywhere (but as I said, probably not a good idea at that point)

Signed-off-by: salaheldinsoliman <salaheldin_sameh@aucegypt.edu>

xermicus

Thanks, LGTM

seanyoung · 2023-12-07T08:44:44Z

@leighmcculloch is this PR ready to be merged? You're listed as a reviewer.

salaheldinsoliman · 2023-12-07T21:43:20Z

@leighmcculloch is this PR ready to be merged? You're listed as a reviewer.

@seanyoung @leighmcculloch is AFK for a week, but I think the PR is ready for merging. If extra stuff is needed they could be addressed in the upcoming PRs

Refs: hyperledger/solang#1602

* Adding the `soroban-copilot` repository to the Stellar ecosystem * Adding the `CometDEX` organization to the Stellar ecosystem * add whalestack gh organization to Stellar ecosystem * add Baseflow's Rust SDK to the Stellar ecosystem * add okashi-dev github org to Stellar ecosystem * add stellar-chef repository * add github organization for platalabs * add CoinFabrik/scout project to Stellar ecosystem * add the Sorosan org to the Stellar ecosystem * add Solang compiler to the Stellar ecosystem Refs: hyperledger/solang#1602 * add the kwickbit organization to the Stellar ecosystem * add the StellarGPT project to the Stellar ecosystem * add a couple BlockScience projects to the Stellar ecosystem * add the Gecko-Fuzz project to the Stellar ecosystem * add some of rahul's projects to the Stellar ecosystem * add the give-credit project to the Stellar ecosystem * add a couple token projects to the Stellar ecosystem * add the GladiusClub organization to the Stellar ecosystem * add the zkbricks project to the Stellar ecosystem * add Excellar to the Stellar ecosystem * add the SorobanPulse project to the Stellar ecosystem * add the BP Ventures organization to the Stellar ecosystem * add the Tellus Cooperative organization to the Stellar ecosystem * add the multiclique protocol repo to the Stellar ecosystem * add a couple projects from Eiger to the Stellar ecosystem * add projects from LinkIO to the Stellar ecosystem * update the TENK-DAO repos in the Stellar ecosystem * add another ScaleMote repo to the Stellar ecosystem * add a Band protocol project to the Stellar ecosystem * add the CometDEX org to the Stellar ecosystem * add a SubQuery repo to the Stellar ecosystem * add a jet protocol repo to the Stellar ecosystem * add the Soroban Learn organization to the Stellar ecosystem * add the AssetDesk organization to the Stellar ecosystem * adding some new Mojoflower repos to the Stellar ecosystem * add the Reflector organization to the Stellar ecosystem * add some repos from Mavennet to the Stellar ecosystem * add some repos from paulfears to the Stellar ecosystem * update Slender repos in the Stellar ecosystem * add some more repos from rahimklaber to the Stellar ecosystem * add a Blue Marble repo to the Stellar ecosystem * Add the Observer organization to the Stellar ecosystem * add an Allbridge repo to the Stellar ecosystem * add the Fluxity organization to the Stellar ecosystem * add the Keizai organization to the Stellar ecosystem * add the Localcoin repo to the Stellar ecosystem * Add the LumosDAO repo to the Stellar ecosystem * add a MicaTechnology repo to the Stellar ecosystem * add a MixiP-io repo to the Stellar ecosystem * add some of Argo Nevis' repos to the Stellar ecosystem * add some more BlockShangerous repos to the Stellar ecosystem * add the Rubic SDK repo to the Stellar ecosystem * add the SeedShare repo to the Stellar ecosystem * add the Sorolana repo to the Stellar ecosystem * add some Meta Soft XLM repos to the Stellar ecosystem * add a couple more Tosin Shada repos to the Stellar ecosystem * add Blockchain wallet to the Stellar ecosystem * add the walletban organization to the Stellar ecosystem * Revert "add CoinFabrik/scout project to Stellar ecosystem" This reverts commit 07c24f3. * Revert "add Solang compiler to the Stellar ecosystem" This reverts commit 86f6760. * Revert "add the Gecko-Fuzz project to the Stellar ecosystem" This reverts commit c8098d9. * Revert "add the zkbricks project to the Stellar ecosystem" This reverts commit eb6a35d. * Revert "add the Localcoin repo to the Stellar ecosystem" This reverts commit b68cc15. * Revert "add the Rubic SDK repo to the Stellar ecosystem" This reverts commit 91030fa. * Revert "add the SeedShare repo to the Stellar ecosystem" This reverts commit 855a26c. * Revert "add Blockchain wallet to the Stellar ecosystem" This reverts commit 0f73bd3. * add the stellar term organization to the Stellar ecosystem * Make Gladius Club a sub-ecosystem of Stellar * change BP Ventures to a sub-ecosystem of Stellar * Change CometDEX to a sub-ecosystem of Stellar * Change paltalabs to be a sub-ecosystem of Stellar * Change Tellus-Cooperative to be a sub-ecosystem of Stellar * Change Whalestack to be a sub-ecosystem of Stellar * Change Obsrvr to be a sub-ecosystem of Stellar * Change AssetDesk to be a sub-ecosystem of Stellar * Change Keizai to a sub-ecosystem of Stellar * Change KwickBit to a sub-ecosystem of Stellar * Change walletban to a sub-ecosystem of Stellar * validate sort order for new sub-ecosystems

salaheldinsoliman requested review from seanyoung, xermicus and LucasSte as code owners November 26, 2023 19:01

LucasSte reviewed Nov 27, 2023

View reviewed changes

xermicus reviewed Nov 28, 2023

View reviewed changes

seanyoung requested changes Nov 29, 2023

View reviewed changes

use mangle_name_contracts in soroban

e9f8d2f

Signed-off-by: salaheldinsoliman <salaheldin_sameh@aucegypt.edu>

salaheldinsoliman force-pushed the soroban_working branch from e63685d to e9f8d2f Compare December 1, 2023 13:27

Merge branch 'hyperledger:main' into soroban_working

6e7eb52

cleanup

8ac8cf5

Signed-off-by: salaheldinsoliman <salaheldin_sameh@aucegypt.edu>

salaheldinsoliman requested a review from seanyoung December 1, 2023 13:56

seanyoung approved these changes Dec 2, 2023

View reviewed changes

added soroban to CHANGELOG.md

6cdcb99

Signed-off-by: salaheldinsoliman <salaheldin_sameh@aucegypt.edu>

salaheldinsoliman requested a review from xermicus December 3, 2023 20:31

xermicus reviewed Dec 5, 2023

View reviewed changes

salaheldinsoliman requested a review from LucasSte December 6, 2023 00:58

xermicus reviewed Dec 6, 2023

View reviewed changes

LucasSte approved these changes Dec 6, 2023

View reviewed changes

salaheldinsoliman force-pushed the soroban_working branch from 43a6ccd to 0298b62 Compare December 6, 2023 18:41

properly add soroban feature flag

43b6142

Signed-off-by: salaheldinsoliman <salaheldin_sameh@aucegypt.edu>

salaheldinsoliman force-pushed the soroban_working branch from 0298b62 to 43b6142 Compare December 6, 2023 19:24

xermicus approved these changes Dec 7, 2023

View reviewed changes

Merge branch 'main' into soroban_working

9cbf17d

seanyoung merged commit e6a2187 into hyperledger:main Dec 8, 2023
18 of 20 checks passed

ElliotFriend added a commit to ElliotFriend/ec-crypto-ecosystems that referenced this pull request Feb 14, 2024

add Solang compiler to the Stellar ecosystem

86f6760

Refs: hyperledger/solang#1602

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Soroban target foundations. #1602

Add Soroban target foundations. #1602

salaheldinsoliman commented Nov 26, 2023

LucasSte Nov 27, 2023

LucasSte Nov 27, 2023

leighmcculloch Nov 28, 2023 •

edited

Loading

LucasSte Nov 27, 2023

xermicus Nov 28, 2023

xermicus Nov 28, 2023

codecov bot commented Nov 28, 2023 •

edited

Loading

seanyoung commented Nov 29, 2023

xermicus commented Nov 29, 2023

salaheldinsoliman commented Dec 1, 2023

seanyoung left a comment

seanyoung commented Dec 2, 2023

salaheldinsoliman commented Dec 3, 2023

xermicus Dec 5, 2023

xermicus Dec 5, 2023

xermicus Dec 6, 2023

graydon Dec 8, 2023

xermicus Dec 6, 2023

xermicus Dec 6, 2023

xermicus Dec 6, 2023

xermicus Dec 6, 2023

xermicus left a comment

seanyoung commented Dec 7, 2023

salaheldinsoliman commented Dec 7, 2023

	// print all elemnts of the CFGName struct, separated by a ::, in this order: contract_name::function_type::function_name
	// print all elements of the CFGName struct, separated by a ::, in this order: contract_name::function_type::function_name

	soroban-sdk = {version = "20.0.0-rc2", features = ["testutils"]}
	soroban-sdk = { version = "20.0.0-rc2", features = ["testutils"] }

Add Soroban target foundations. #1602

Add Soroban target foundations. #1602

Conversation

salaheldinsoliman commented Nov 26, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

leighmcculloch Nov 28, 2023 • 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

codecov bot commented Nov 28, 2023 • edited Loading

Codecov Report

seanyoung commented Nov 29, 2023

xermicus commented Nov 29, 2023

salaheldinsoliman commented Dec 1, 2023

seanyoung left a comment

Choose a reason for hiding this comment

seanyoung commented Dec 2, 2023

salaheldinsoliman commented Dec 3, 2023

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

xermicus left a comment

Choose a reason for hiding this comment

seanyoung commented Dec 7, 2023

salaheldinsoliman commented Dec 7, 2023

leighmcculloch Nov 28, 2023 •

edited

Loading

codecov bot commented Nov 28, 2023 •

edited

Loading