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

Add method name to execution message and call syscall #112

Merged
merged 16 commits into from
Oct 4, 2024
Merged

Conversation

lrettig
Copy link
Contributor

@lrettig lrettig commented Oct 1, 2024

Closes #111

@lrettig lrettig requested review from jellonek and poszu October 1, 2024 23:39
vm/sdk/src/lib.rs Outdated Show resolved Hide resolved
ffi/athcon/bindings/rust/athcon-vm/src/lib.rs Outdated Show resolved Hide resolved
ffi/athcon/bindings/rust/athcon-vm/src/lib.rs Show resolved Hide resolved
ffi/athcon/bindings/rust/athcon-vm/src/lib.rs Outdated Show resolved Hide resolved
ffi/athcon/include/athcon/athcon.h Show resolved Hide resolved
Factor in review comments
Integration test passing
Move call syscall implementation to new location
Finish updating host fn signatures
Adding entrypoint test
tests/entrypoint/src/main.rs Outdated Show resolved Hide resolved
vm/sdk/src/lib.rs Outdated Show resolved Hide resolved
@lrettig lrettig marked this pull request as ready for review October 3, 2024 22:59
@lrettig lrettig requested a review from poszu October 3, 2024 22:59
Comment on lines 29 to 32
#[allow(dead_code)]
fn test3() {
// no-op
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

What's the point of having it?

Suggested change
#[allow(dead_code)]
fn test3() {
// no-op
}

Copy link
Contributor Author

@lrettig lrettig Oct 4, 2024

Choose a reason for hiding this comment

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

the point is that this test should fail:

https://github.com/athenavm/athena/pull/112/files#diff-db7734ca10e82c440f78ad4c35a8a418dabdb4afe27289c8aadd86e406899a59R208-R226

athena/runner/src/vm.rs

Lines 208 to 226 in 74b036a

// this will execute a specific method
let result = AthenaVm::new().execute(
&mut host,
AthenaRevision::AthenaFrontier,
AthenaMessage::new(
MessageKind::Call,
0,
1000000,
Address::default(),
Address::default(),
None,
Some("athexp_test3".as_bytes().to_vec()),
Balance::default(),
vec![],
),
elf,
);
// this should fail, as this method is not `callable`
assert_eq!(result.status_code, StatusCode::Failure);

In other words, a method not marked #[callable] shouldn't be callable

Copy link
Collaborator

Choose a reason for hiding this comment

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

It sounds more like a test of the procedural macro (which would check that methods not marked #[callable] are not exported in the macro). This test here would (should) fail even fn test3() was removed, right (it wouldn't find the method)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

correct, but that would be testing something different :) if you think these tests should be restructured/moved, we can do that.

ffi/athcon/bindings/go/athcon.go Outdated Show resolved Hide resolved
ffi/athcon/bindings/rust/athcon-client/src/host.rs Outdated Show resolved Hide resolved
@lrettig lrettig requested a review from poszu October 4, 2024 16:49
@lrettig lrettig merged commit 8d4646a into main Oct 4, 2024
8 checks passed
@lrettig lrettig deleted the method-name branch October 4, 2024 20:47
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.

Allow multiple program entry points over FFI
2 participants