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.

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

Soroban counter.sol example #1645

Merged

Conversation

salaheldinsoliman
Copy link
Contributor

@salaheldinsoliman salaheldinsoliman commented May 18, 2024

This PR aims to make Solang support a simple counter.sol example on Soroban, where a storage variable is instatiated, modified and retrieved. The counter contract is only limited to uint64 data types, and only supports instance soroban storage.

This can be considered a "skeleton" for supporting more data and storage types, as well as more host function invokations.

  • Support Soroban storage function calls put_contract_data, get_contract_data and has_contract_data
  • Implement a wrapper init for storage_initializer
  • Implement wrappers for public functions
  • Insert decoding/encoding instructions into the wrapper functions
  • Soroban doesn't have function return codes. This needs to be handled all over emit
  • Add integration tests and MockVm tests

salaheldinsoliman and others added 26 commits May 8, 2024 16:11
Signed-off-by: salaheldinsoliman <[email protected]>
Signed-off-by: salaheldinsoliman <[email protected]>
Signed-off-by: salaheldinsoliman <[email protected]>
Signed-off-by: salaheldinsoliman <[email protected]>
Signed-off-by: salaheldinsoliman <[email protected]>
Signed-off-by: salaheldinsoliman <[email protected]>
Signed-off-by: salaheldinsoliman <[email protected]>
Signed-off-by: salaheldinsoliman <[email protected]>
Signed-off-by: salaheldinsoliman <[email protected]>
Signed-off-by: salaheldinsoliman <[email protected]>
Signed-off-by: salaheldinsoliman <[email protected]>
Signed-off-by: salaheldinsoliman <[email protected]>
Signed-off-by: salaheldinsoliman <[email protected]>
Signed-off-by: salaheldinsoliman <[email protected]>
Signed-off-by: salaheldinsoliman <[email protected]>
Signed-off-by: salaheldinsoliman <[email protected]>
Signed-off-by: salaheldinsoliman <[email protected]>
Signed-off-by: salaheldinsoliman <[email protected]>
Signed-off-by: salaheldinsoliman <[email protected]>
Signed-off-by: salaheldinsoliman <[email protected]>
@salaheldinsoliman salaheldinsoliman marked this pull request as ready for review June 14, 2024 15:23
Copy link
Contributor

@seanyoung seanyoung left a comment

Choose a reason for hiding this comment

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

Looks great

src/bin/cli/mod.rs Show resolved Hide resolved
src/codegen/dispatch/mod.rs Outdated Show resolved Hide resolved
src/codegen/dispatch/soroban.rs Outdated Show resolved Hide resolved
src/codegen/dispatch/soroban.rs Outdated Show resolved Hide resolved
src/codegen/dispatch/soroban.rs Outdated Show resolved Hide resolved
src/emit/instructions.rs Outdated Show resolved Hide resolved
bin.builder
.build_conditional_branch(success, success_block, bail_block)
.unwrap();
// Soroban doesnt have return codes, and only returns a single i64 value
Copy link
Contributor

Choose a reason for hiding this comment

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

I was wondering if this should be enforced in sema; for Soroban, we could simply give an error diagnostic if a function returns anything else but an int64.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What is reffered to here is the wasm functions, not Solidity functions. That is, if a Solidity function returns nothing, the wasm function returns an i64 value to be XDR decoded as Void.

Copy link
Contributor

Choose a reason for hiding this comment

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

Right, the comment could with explaining that

src/codegen/dispatch/soroban.rs Outdated Show resolved Hide resolved
integration/soroban/test_helpers.js Outdated Show resolved Hide resolved
src/codegen/dispatch/soroban.rs Show resolved Hide resolved
src/emit/binary.rs Outdated Show resolved Hide resolved
src/emit/instructions.rs Outdated Show resolved Hide resolved
src/emit/instructions.rs Outdated Show resolved Hide resolved
src/emit/soroban/mod.rs Outdated Show resolved Hide resolved
src/emit/soroban/mod.rs Outdated Show resolved Hide resolved
src/emit/soroban/mod.rs Outdated Show resolved Hide resolved
src/emit/soroban/target.rs Show resolved Hide resolved
fn print(&self, bin: &Binary, string: PointerValue, length: IntValue) {
unimplemented!()
}
fn print(&self, bin: &Binary, string: PointerValue, length: IntValue) {}
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this function be doing nothing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When there is a runtime error, an overflow maybe, a call to this function is made. The ideal scenario is to make a call to Soroban's host function responsible for logging, but since this is out of scope for this PR, I left it out.

A //TODO might be better here

Signed-off-by: salaheldinsoliman <[email protected]>
Copy link

codecov bot commented Jun 22, 2024

Codecov Report

Attention: Patch coverage is 92.87671% with 26 lines in your changes missing coverage. Please review.

Project coverage is 88.58%. Comparing base (08dbe49) to head (1a8cb27).

Files Patch % Lines
src/linker/soroban_wasm.rs 80.00% 9 Missing ⚠️
src/emit/soroban/mod.rs 93.00% 7 Missing ⚠️
src/emit/instructions.rs 89.13% 5 Missing ⚠️
src/emit/soroban/target.rs 93.02% 3 Missing ⚠️
src/codegen/dispatch/soroban.rs 98.23% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1645      +/-   ##
==========================================
+ Coverage   88.51%   88.58%   +0.07%     
==========================================
  Files         163      164       +1     
  Lines       72869    73152     +283     
==========================================
+ Hits        64497    64802     +305     
+ Misses       8372     8350      -22     

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

@salaheldinsoliman salaheldinsoliman merged commit 399c199 into hyperledger:main Jun 25, 2024
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants