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

feat: Add new syscalls and move to separate crate #507

Merged
merged 11 commits into from
Aug 12, 2021
Merged

Conversation

austinabell
Copy link
Contributor

@austinabell austinabell commented Jul 29, 2021

  • Adds new syscalls
  • Moves sys to own crate, to be more easily shared and used externally

There was one function I didn't add, which is gas(u64) which just consumes gas, as I didn't see any benefit. Happy to add if anyone can think of a reason, but I don't see any usage of it anywhere.

I won't release this crate until this is approved, just to be safe

@austinabell
Copy link
Contributor Author

austinabell commented Jul 29, 2021

new clippy version causes warnings, will fix in a separate PR (#508)

@mikedotexe
Copy link
Contributor

Is this part of the broader goal to have an efficient "under layer" to the SDK so folks can optionally build no_std contracts?

@austinabell
Copy link
Contributor Author

Is this part of the broader goal to have an efficient "under layer" to the SDK so folks can optionally build no_std contracts?

Yes, it is. Let me link the issue here: #463

@@ -31,16 +34,25 @@ extern "C" {
pub fn sha256(value_len: u64, value_ptr: u64, register_id: u64);
pub fn keccak256(value_len: u64, value_ptr: u64, register_id: u64);
pub fn keccak512(value_len: u64, value_ptr: u64, register_id: u64);
pub fn ripemd160(value_len: u64, value_ptr: u64, register_id: u64);
pub fn ecrecover(
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we can't just link the docs from nearcore directly into these sys calls. I feel like it would be a painpoint eventually for someone who's trying to optimize their contracts but not directly know what all these syscalls are, and have to indirectly discover near-vm-logic/logic.rs to understand them

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah definitely would be some value add here. Definitely a niche use case for someone to be using syscalls directly, but I will open an issue for this

Copy link
Member

@ChaoticTempest ChaoticTempest left a comment

Choose a reason for hiding this comment

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

LGTM: didn't see anything out of the ordinary when cross referencing them with nearcore

@austinabell austinabell mentioned this pull request Aug 12, 2021
@austinabell austinabell merged commit 1a706b2 into master Aug 12, 2021
@austinabell austinabell deleted the austin/sys_pull branch August 12, 2021 19:29
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