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.

Sign up for GitHub

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

Jump to bottom

Extract entrypoint crate #2430

Merged
merged 7 commits into from
Oct 15, 2024
Merged

Extract entrypoint crate #2430

merged 7 commits into from
Oct 15, 2024
+70 −22

Conversation

kevinheavey
Copy link

Problem

entrypoint.rs needs to be made available outside solana_program

This branches off #2429 so that needs to be merged first.

Summary of Changes

  • Move entrypoint.rs to its own crate and re-export in solana-program for backwards compatibility
  • Re-export solana_msg::msg in the new crate so that the custom_heap_default macro still compiles reliably

Fun milestone: this tower of PRs is enough to compile a hello-world program without using solana_program. On my machine it takes 1.75 seconds to compile such a program, while it takes 23 seconds to compile using solana-program 2.0

@kevinheavey kevinheavey force-pushed the extract-entrypoint branch 2 times, most recently from 7b452ab to 86d5210 Compare August 3, 2024 14:18
@kevinheavey kevinheavey mentioned this pull request Aug 3, 2024
Closed
@mergify Mergify
Copy link

mergify bot commented Aug 9, 2024

The Firedancer team maintains a line-for-line reimplementation of the
native programs, and until native programs are moved to BPF, those
implementations must exactly match their Agave counterparts.
If this PR represents a change to a native program implementation (not
tests), please include a reviewer from the Firedancer team. And please
keep refactors to a minimum.

@kevinheavey kevinheavey marked this pull request as ready for review September 25, 2024 09:39
@kevinheavey kevinheavey force-pushed the extract-entrypoint branch 2 times, most recently from 7c4ce1d to d2a8514 Compare September 26, 2024 09:37
joncinque
joncinque reviewed Oct 15, 2024
View reviewed changes
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.

Just a couple of nits, then this should be good to go!

sdk/entrypoint/Cargo.toml Outdated
@@ -0,0 +1,19 @@
[package]
name = "solana-entrypoint"
Copy link

Choose a reason for hiding this comment

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

nit: what do you think about solana-program-entrypoint for the name? I think that'll make it a bit clearer, since the term "entrypoint" is also used for gossip

sdk/entrypoint/src/lib.rs Outdated
pub use {solana_account_info::MAX_PERMITTED_DATA_INCREASE, solana_program_error::ProgramResult};
// need to re-export msg for custom_heap_default macro
pub use {
solana_account_info::MAX_PERMITTED_DATA_INCREASE, solana_msg::msg as __msg,
Copy link

Choose a reason for hiding this comment

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

nit: Is there a reason for the double underscore vs just one? We don't seem to use the double-underscore anywhere for private members, so how about just _msg?

Copy link
Author

Choose a reason for hiding this comment

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

It indicates that something is technically public but shouldn't be used externally https://users.rust-lang.org/t/when-to-name-a-mod-starting-with-2-underscore/85953

Copy link

Choose a reason for hiding this comment

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

Gotcha, works for me!

joncinque
joncinque reviewed Oct 15, 2024
View reviewed changes
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 to me! @yihau can you accept ownership of solana-program-entrypoint?

@yihau
Copy link
Member

yihau commented Oct 15, 2024

✅

@kevinheavey kevinheavey added automerge automerge Merge this Pull Request automatically once CI passes and removed automerge automerge Merge this Pull Request automatically once CI passes labels Oct 15, 2024
joncinque
joncinque previously approved these changes Oct 15, 2024
View reviewed changes
@joncinque joncinque added the automerge automerge Merge this Pull Request automatically once CI passes label Oct 15, 2024
@anza-team anza-team removed the automerge automerge Merge this Pull Request automatically once CI passes label Oct 15, 2024
@anza-team
Copy link
Collaborator

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

@kevinheavey
Copy link
Author

Had to fix some conflicts, could you approve again @joncinque?

@joncinque joncinque added the automerge automerge Merge this Pull Request automatically once CI passes label Oct 15, 2024
@mergify Mergify
Copy link

mergify bot commented Oct 15, 2024

automerge label removed due to a CI failure

@mergify mergify bot removed the automerge automerge Merge this Pull Request automatically once CI passes label Oct 15, 2024
@joncinque joncinque merged commit 598f7ae into anza-xyz:master Oct 15, 2024
55 checks passed
ray-kast pushed a commit to abklabs/agave that referenced this pull request Nov 27, 2024
@kevinheavey @ray-kast
Extract entrypoint crate (anza-xyz#2430)
98470b8 
* extract entrypoint crate

* missing re-export

* fix path

* fmt

* rename to solana-program-entrypoint

* move to program-entrypoint dir

* update lock files
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joncinque joncinque joncinque approved these changes

Assignees
No one assigned
Labels
need:merge-assist
Projects
SDK Breakup + Breakout
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@kevinheavey @yihau @anza-team @joncinque