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

feat: populate metadata with build information #55

Merged
merged 6 commits into from
Oct 20, 2022
Merged

feat: populate metadata with build information #55

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9d4672b
populate metadata with build information
itegulov Sep 21, 2022
2dc8d9f
Merge remote-tracking branch 'origin/main' into daniyar/metadata-with…
itegulov Oct 20, 2022
9b454d5
include build info into the embedded abi
itegulov Oct 20, 2022
258e7fe
migrate integrations tests to near-abi 0.2.0
itegulov Oct 20, 2022
758c7a4
Merge remote-tracking branch 'origin/main' into daniyar/metadata-with…
itegulov Oct 20, 2022
0c2bb6f
fix typo
itegulov Oct 20, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 4 additions & 1 deletion cargo-near/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,18 @@ categories = ["development-tools", "development-tools::cargo-plugins", "developm

[dependencies]
anyhow = "1.0"
bs58 = "0.4"
camino = "1.1.1"
cargo_metadata = "0.14"
clap = { version = "3.2", features = ["derive", "env"] }
colored = "2.0"
env_logger = "0.9"
log = "0.4"
rustc_version = "0.4"
serde_json = "1.0"
sha2 = "0.10"
symbolic-debuginfo = "8.8"
schemars = "0.8"
near-abi = { version = "0.1.0-pre.0", features = ["__chunked-entries"] }
near-abi = { version = "0.2.0", features = ["__chunked-entries"] }
libloading = "0.7.3"
zstd = "0.11"
2 changes: 2 additions & 0 deletions cargo-near/src/abi.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,8 @@ fn extract_metadata(crate_metadata: &CrateMetadata) -> near_abi::AbiMetadata {
name: Some(package.name.clone()),
version: Some(package.version.to_string()),
authors: package.authors.clone(),
build: None,
wasm_hash: None,
other: HashMap::new(),
}
}
Expand Down
33 changes: 21 additions & 12 deletions cargo-near/src/build.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ use crate::abi::{AbiCompression, AbiFormat, AbiResult};
use crate::cargo::{manifest::CargoManifestPath, metadata::CrateMetadata};
use crate::{abi, util, BuildCommand};
use colored::Colorize;
use near_abi::BuildInfo;
use sha2::{Digest, Sha256};
use std::io::BufRead;

const COMPILATION_TARGET: &str = "wasm32-unknown-unknown";
Expand Down Expand Up @@ -35,18 +37,15 @@ pub(crate) fn run(args: BuildCommand) -> anyhow::Result<()> {
cargo_args.push("--release");
}

let mut pretty_abi_path = None;
let mut abi = None;
let mut min_abi_path = None;
if !args.no_abi {
let contract_abi = abi::generate_abi(&crate_metadata, args.doc, true)?;
let AbiResult { path } = abi::write_to_file(
&contract_abi,
&crate_metadata,
AbiFormat::Json,
AbiCompression::NoOp,
)?;
pretty_abi_path.replace(util::copy(&path, &out_dir)?);

let mut contract_abi = abi::generate_abi(&crate_metadata, args.doc, true)?;
contract_abi.metadata.build = Some(BuildInfo {
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't we do this before writing out the embedded version of the ABI?

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 is opinionated I guess. Do you think this information should be embedded inside of the wasm file?

Copy link
Contributor

Choose a reason for hiding this comment

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

Unsure, but this, in a sense, is part of the contract's signature.

We could make it opt-in, though, so another flag for including build information in the embedded ABI?

near/near-abi-rs#8 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, decided to include the build information in the end. It is only ~30 extra bytes anyway and we can always come up with a flag that disables it if there is a demand.

Also, probably wise to try and keep embedded and non-embedded ABI to look as close as possible.

compiler: format!("rustc {}", rustc_version::version()?),
builder: format!("cargo-near {}", env!("CARGO_PKG_VERSION")),
image: None,
Copy link
Contributor

Choose a reason for hiding this comment

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

Curious how we're going to inject this bit of information. Env var only set when built from contract-builder?

Because either way, that information needs to be known at this stage to be embedded in the wasm file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Env var only set when built from contract-builder?

Most likely. There no built-in way to get the docker image name from inside of the container from what I have researched.

Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if sdk-js gets an equivalent of contract-builder at some point and what that would mean for this field. I guess in a sense, the builder information plus the image helps better describe what type of contract-builder this is. But it's a non-issue, we have a long way ahead before we're at that point.

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 was actually wondering if it makes sense to "demote" contract-builder to only serve as an operating system and then just install whatever is necessary based on the build info (e.g. rustc 1.61.0, cargo-near 0.2.0). This way it could potentially work for both sdk-rs and sdk-js.

Also, presumably sometime in the future rustc/cargo will reach the point where wasm builds can be bit-by-bit reproducible and hedonistic and we will not need an image anymore.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is a good point, not sure what the size overhead would be from including the JS SDK in the same docker image, but it would certainly help unify things. Also, imho, the current docker image might be a little too big, I'll try to see if we can cut things down a bit, drop unnecessary binaries, strip some maybe etc. Doesn't seem like the official rust image makes much of an effort to reduce size anyway.

});
if args.embed_abi {
let path = util::handle_step("Compressing ABI to be embedded..", || {
let AbiResult { path } = abi::write_to_file(
Expand All @@ -59,6 +58,7 @@ pub(crate) fn run(args: BuildCommand) -> anyhow::Result<()> {
})?;
min_abi_path.replace(util::copy(&path, &out_dir)?);
}
abi = Some(contract_abi);
}

if let (true, Some(abi_path)) = (args.embed_abi, &min_abi_path) {
Expand All @@ -82,8 +82,17 @@ pub(crate) fn run(args: BuildCommand) -> anyhow::Result<()> {
"Binary",
wasm_artifact.path.to_string().bright_yellow().bold(),
)];
if let Some(abi_path) = pretty_abi_path {
messages.push(("ABI", abi_path.to_string().yellow().bold()));
if let Some(mut abi) = abi {
let mut hasher = Sha256::new();
hasher.update(std::fs::read(&wasm_artifact.path)?);
let hash = hasher.finalize();
let hash = bs58::encode(hash).into_string();
abi.metadata.wasm_hash = Some(hash);

let AbiResult { path } =
abi::write_to_file(&abi, &crate_metadata, AbiFormat::Json, AbiCompression::NoOp)?;
let pretty_abi_path = util::copy(&path, &out_dir)?;
messages.push(("ABI", pretty_abi_path.to_string().yellow().bold()));
}
if let Some(abi_path) = min_abi_path {
messages.push(("Embedded ABI", abi_path.to_string().yellow().bold()));
Expand Down
2 changes: 1 addition & 1 deletion integration-tests/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ edition = "2021"
publish = false

[dependencies]
near-abi = "0.1.0-pre.0"
near-abi = "0.2.0"

[dev-dependencies]
anyhow = "1.0"
Expand Down
2 changes: 1 addition & 1 deletion integration-tests/templates/_Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ edition = "2021"
crate-type = ["cdylib"]

[dependencies]
near-sdk = { version = "4.1.0-pre.3", features = ["abi"] }
near-sdk = { version = "4.1.0-pre.3", features = ["abi"], git = "https://github.com/near/near-sdk-rs.git", rev = "6d73c9ff4fd095fc23eaa000c14ab65c15c4aa6b" }
serde = { version = "1", features = ["derive"] }
schemars = "0.8"

Expand Down
2 changes: 2 additions & 0 deletions integration-tests/templates/sdk-dependency/_Cargo_explicit.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ schemars = "0.8"
[dependencies.near-sdk]
version = "4.1.0-pre.3"
features = ["abi"]
git = "https://github.com/near/near-sdk-rs.git"
rev = "6d73c9ff4fd095fc23eaa000c14ab65c15c4aa6b"

[workspace]
members = []
Expand Down
2 changes: 1 addition & 1 deletion integration-tests/templates/sdk-dependency/_Cargo_multiple_features.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ edition = "2021"
crate-type = ["cdylib"]

[dependencies]
near-sdk = { version = "4.1.0-pre.3", features = ["abi", "unstable"] }
near-sdk = { version = "4.1.0-pre.3", features = ["abi", "unstable"], git = "https://github.com/near/near-sdk-rs.git", rev = "6d73c9ff4fd095fc23eaa000c14ab65c15c4aa6b" }
serde = { version = "1", features = ["derive"] }
schemars = "0.8"

Expand Down
2 changes: 1 addition & 1 deletion integration-tests/templates/sdk-dependency/_Cargo_no_default_features.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ edition = "2021"
crate-type = ["cdylib"]

[dependencies]
near-sdk = { version = "4.1.0-pre.3", default-features = false, features = ["abi"] }
near-sdk = { version = "4.1.0-pre.3", default-features = false, features = ["abi"], git = "https://github.com/near/near-sdk-rs.git", rev = "6d73c9ff4fd095fc23eaa000c14ab65c15c4aa6b" }
serde = { version = "1", features = ["derive"] }
schemars = "0.8"

Expand Down
2 changes: 1 addition & 1 deletion integration-tests/templates/sdk-dependency/_Cargo_patch.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ serde = { version = "1", features = ["derive"] }
schemars = "0.8"

[patch.crates-io]
near-sdk = { git = "https://github.com/near/near-sdk-rs.git", rev = "792d5eb26d26a0878dbf59e304afa4e19540c317" }
near-sdk = { git = "https://github.com/near/near-sdk-rs.git", rev = "792d5eb26d26a0878dbf59e304afa4e19540c317", git = "https://github.com/near/near-sdk-rs.git", rev = "6d73c9ff4fd095fc23eaa000c14ab65c15c4aa6b" }
Copy link
Contributor

Choose a reason for hiding this comment

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

lol 😅

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hahaha, interesting that this is still a valid toml syntax

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 am working on a PR right now that will allow us not to update all the toml files every time we upgrade the SDK version, so will fix there

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice! Can't wait to see it.


[workspace]
members = []
Expand Down
4 changes: 2 additions & 2 deletions integration-tests/templates/sdk-dependency/_Cargo_platform_specific.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@ serde = { version = "1", features = ["derive"] }
schemars = "0.8"

[target.'cfg(windows)'.dependencies]
near-sdk = { version = "4.1.0-pre.3", features = ["abi"] }
near-sdk = { version = "4.1.0-pre.3", features = ["abi"], git = "https://github.com/near/near-sdk-rs.git", rev = "6d73c9ff4fd095fc23eaa000c14ab65c15c4aa6b" }

[target.'cfg(unix)'.dependencies]
near-sdk = { version = "4.1.0-pre.3", features = ["abi"] }
near-sdk = { version = "4.1.0-pre.3", features = ["abi"], git = "https://github.com/near/near-sdk-rs.git", rev = "6d73c9ff4fd095fc23eaa000c14ab65c15c4aa6b" }

[workspace]
members = []
Expand Down
2 changes: 1 addition & 1 deletion integration-tests/templates/sdk-dependency/_Cargo_renamed.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ serde = { version = "1", features = ["derive"] }
schemars = "0.8"

[dependencies.near]
near = { version = "4.1.0-pre.3", package = "near-sdk", features = ["abi"] }
near = { version = "4.1.0-pre.3", package = "near-sdk", features = ["abi"], git = "https://github.com/near/near-sdk-rs.git", rev = "6d73c9ff4fd095fc23eaa000c14ab65c15c4aa6b" }

[workspace]
members = []
Expand Down
Loading