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

WIP: Try killing declare_id! by moving it into the #[program] macro. #868

Closed
wants to merge 6 commits into from

Conversation

cqfd
Copy link
Contributor

@cqfd cqfd commented Oct 11, 2021

This is me attempting to kill the declare_id! macro/fix #695 by moving that ID declaration machinery inside #[program].

I first tried implementing a program_id!() macro, but I think it's actually a bit tricky since proc macros don't really give you access to the file in which they're invoked :/ There's https://docs.rs/proc-macro2/0.4.30/proc_macro2/struct.Span.html#method.source_file but I couldn't figure out how to actually get access to it (it's "semver exempt" and you have to go out of your way to turn it on, as far as I understand it). This is a bummer because it means if you have multiple programs, the individual program_id!() calls don't really know which program they're in.

So, this PR just sneaks the ID declaration into the #[program] macro—which conveniently knows the program's name, so I don't have to care which file I'm in.

.map(|s| s.parse::<toml::Value>())
.unwrap()
.unwrap();
let cluster = std::env::var("ANCHOR_CLUSTER").unwrap_or("localnet".to_string());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

^ This env var is WIP, I was thinking the anchor command could feed it along if you specify --provider.cluster?

Copy link
Member

Choose a reason for hiding this comment

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

sgtm

use crate::Program;

pub fn generate(program: &Program) -> proc_macro2::TokenStream {
let anchor_toml = std::fs::read_to_string("./Anchor.toml")
Copy link
Member

Choose a reason for hiding this comment

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

What should happen if the program id isn't in the Anchor.toml?

I think it would convenient if it looked for the default deploy key in the local filesystem. I.e. target/deploy/<program-name>-keypair.json, which is what solana program deploy uses by default. Then everything would work by default, i.e., you can clone, test, and deploy without any modifications--since you would have the keypair locally for the deploy.

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, was wondering about all the error handling 😬

So the desired behavior is: check for the id in Anchor.toml first, otherwise fall back to target/deploy/-keypair.json? (I realized working on this that I need to learn more about the Anchor.toml format, what each part means/entails etc.)

Copy link
Contributor Author

@cqfd cqfd Oct 11, 2021

Choose a reason for hiding this comment

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

Having an issue including solana-sdk as a dependency so this macro can use Keypair, when I try to use anchor build etc. I get a ring 0.16.20 build error. Not sure why yet (maybe because anchor build uses build-bpf whereas other places that use solana-sdk, e.g. the cli, don't?).

Will try again tomorrow.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

^ Have gotten around that for now by just snagging the final 32 bytes of the on-disk keypair and bs58-ing it 🙃

@Arrowana
Copy link
Contributor

wen no declare_id

@@ -637,7 +637,7 @@ fn build_cwd(
Some(p) => std::env::set_current_dir(&p)?,
};
match verifiable {
false => _build_cwd(idl_out, idl_ts_out, cargo_args),
false => _build_cwd(idl_out, idl_ts_out, cargo_args, cfg),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess I need to thread the cfg.provider.cluster through the docker stuff too, hmm.

std::process::Command::new("touch")
.arg("-m")
.arg("src/lib.rs")
.output()?;
Copy link
Contributor Author

@cqfd cqfd Oct 15, 2021

Choose a reason for hiding this comment

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

Ugh. This seems so dumb, let me know if anyone has a nicer way!

To explain the issue further: if you don't force cargo to rerun macros, imagine you do anchor build --provider.cluster localnet followed by anchor build --provider.cluster mainnet, without making any code changes; without a hack like above, cargo might only actually run macros during the first step :/, and then would reuse the localnet program_id during the second step ://

// Keypair::read_keypair_file, but I can't seem to get it to work as
// a dependency for this crate :/ So just parse the pubkey manually.
let bytes: Vec<u8> = serde_json::from_str(&std::fs::read_to_string(path).ok()?).ok()?;
let pubkey_bytes = &bytes[bytes.len() - 32..];
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Curious if this is just an M1 issue (@davoclavo tried doing a similar thing on his M1 and hit the same ring compilation error I get). But at any rate, I'm assuming pubkey lengths aren't going to change any time soon..?

@armaniferrante
Copy link
Member

Closing as stale.

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.

lang: Add new program_id! macro
3 participants