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

Extract define-syscall crate #1827

Merged
merged 9 commits into from
Jul 2, 2024

Conversation

kevinheavey
Copy link

Problem

  • We want to use the define_syscall macro without having solana-program as a dependency
  • solana-poseidon is already defining the sol_poseidon syscall without using the define_syscall macro, and leaving out the static-syscalls part

Summary of Changes

  • Make a new solana-define-syscall crate and move the define_syscall macro here
  • Use solana_define_syscall::define_syscall in solana-poseidon

Copy link

@joncinque joncinque left a comment

Choose a reason for hiding this comment

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

Looks good! Just one bit to re-export, then we can land this

sdk/program/src/syscalls/definitions.rs Show resolved Hide resolved
define-syscall/src/lib.rs Outdated Show resolved Hide resolved
joncinque
joncinque previously approved these changes Jun 27, 2024
Copy link

@joncinque joncinque 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 to me! Please pass over the crate ownership, and once the crate check passes, we can merge this in. Thanks as always!

@kevinheavey
Copy link
Author

@yihau can you accept the crate ownership? 🙌

@yihau
Copy link
Member

yihau commented Jul 1, 2024

@kevinheavey sure! added!

@kevinheavey kevinheavey force-pushed the define-syscall-crate branch from 563f7d4 to c99b289 Compare July 1, 2024 08:40
@kevinheavey kevinheavey added the automerge automerge Merge this Pull Request automatically once CI passes label Jul 2, 2024
@kevinheavey kevinheavey force-pushed the define-syscall-crate branch from c99b289 to 67f4ae1 Compare July 2, 2024 14:02
@anza-team anza-team removed the automerge automerge Merge this Pull Request automatically once CI passes label Jul 2, 2024
@anza-team
Copy link
Collaborator

😱 New commits were pushed while the automerge label was present.

@kevinheavey kevinheavey force-pushed the define-syscall-crate branch from 67f4ae1 to bdc8d2c Compare July 2, 2024 19:53
@joncinque joncinque added the automerge automerge Merge this Pull Request automatically once CI passes label Jul 2, 2024
@mergify mergify bot merged commit 8eef73d into anza-xyz:master Jul 2, 2024
53 checks passed
@kevinheavey kevinheavey deleted the define-syscall-crate branch July 3, 2024 05:31
samkim-crypto pushed a commit to samkim-crypto/agave that referenced this pull request Jul 31, 2024
* move define_syscall macro to its own crate

* use solana-define-syscall in solana-poseidon

* add missing dep to solana-program

* update crate version to match workspace

* macro hygiene fix

* fix define_syscall call

* re-export sys_hash in solana-program for backwards compatibility

* convert tabs to spaces

* put re-export behind #[cfg(target_feature = "static-syscalls")]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge automerge Merge this Pull Request automatically once CI passes need:merge-assist
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants