hyperledger-solang / solang

Solidity Compiler for Solana and Polkadot
https://solang.readthedocs.io/
Apache License 2.0
1.27k stars 215 forks source link

Add Soroban target foundations. #1602

Closed salaheldinsoliman closed 11 months ago

salaheldinsoliman commented 11 months ago

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.

codecov[bot] commented 11 months ago

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 :warning:
src/emit/soroban/mod.rs 98.60% 2 Missing :warning:
src/bin/cli/mod.rs 0.00% 1 Missing :warning:
src/codegen/events/mod.rs 0.00% 1 Missing :warning:
src/emit/binary.rs 87.50% 1 Missing :warning:
src/lib.rs 66.66% 1 Missing :warning:
src/linker/mod.rs 80.00% 1 Missing :warning:
src/sema/yul/builtin.rs 0.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ 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 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

seanyoung commented 11 months ago

@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 commented 11 months ago

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

salaheldinsoliman commented 11 months ago

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

seanyoung commented 11 months ago

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

salaheldinsoliman commented 11 months ago

@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.

seanyoung commented 11 months ago

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

salaheldinsoliman commented 11 months ago

@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