NEP-330 extension: Build details extension

[Canvinus]

NEP-330: Source Metadata

1.2.0 - Build Details Extension

Overview

This update introduces build details to the contract metadata, containing necessary information about how the contract was built. This makes it possible for others to reproduce the same WASM of this contract. The idea first appeared in the cargo-near SourceScan integration thread.

Benefits

This NEP extension gives developers the capability to save all the required build details, making it possible to reproduce the same WASM code in the future. This ensures greater consistency in contracts and the ability to verify source code. With the assistance of tools like SourceScan and cargo-near, the development process on NEAR becomes significantly easier.

[frol]

Thank you @Canvinus for submitting this NEP.

As a moderator, I reviewed this NEP and it meets the proposed template guidelines. I am moving this NEP to the REVIEW stage and would like to ask the @near/wg-contract-standards working group members to assign 2 Technical Reviewers to complete a technical review (see expectations below).

Just for clarity, Technical Reviewers play a crucial role in scaling NEAR ecosystem as they provide their in-depth expertise in the niche topic while work group members can stay on guard of the NEAR ecosystem. The discussions may get too deep and it would be inefficient for each WG member to dive into every single comment, so NEAR Developer Governance designed this process that includes subject matter experts helping us to scale by writing a summary with the raised concerns and how they were addressed.

Technical Review Guidelines

• First, review the proposal within one week. If you have any suggestions that could be fixed, leave them as comments to the author. It may take a couple of iterations to resolve any open comments.

• Second, once all the suggestions are addressed, produce a Technical Summary, which helps the working group members make a weighted decision faster. Without the summary, the working group will have to read the whole discussion and potentially miss some details.

Technical Summary guidelines:

• A recommendation for the working group if the NEP is ready for voting (it could be approving or rejecting recommendation). Please note that this is the reviewer's personal recommendation.

• A summary of benefits that surfaced in previous discussions. This should include a concise list of all the benefits that others raised, not just the ones that the reviewer personally agrees with.

• A summary of concerns or blockers, along with their current status and resolution. Again, this should reflect the collective view of all commenters, not just the reviewer's perspective.

Here is a nice example and a template for your convenience:

### Recommendation

Add recommendation

### Benefits

* Benefit

* Benefit

### Concerns

| # | Concern | Resolution | Status |

| - | - | - | - |

| 1 | Concern | Resolution | Status |

| 2 | Concern | Resolution | Status |

Please tag the @near/nep-moderators once you are done, so we can move this NEP to the voting stage. Thanks again.

[lachlanglen]

This looks useful. Any thoughts on adding commit_hash while you're at it to remove ambiguity of version field?

version: a string that references the specific commit hash or version of the code currently deployed on-chain.

Having a field that might be one thing or something else is not super clear IMO. I personally have added commit_hash in my own implementation of this standard for my contracts, as I find it useful to have both a version (e.g. v1.0.0) and a commit hash.

I guess a downside here is that it wouldn't be backwards-compatible in instances where the version field has been populated with a commit hash.

[Canvinus]

I totally agree with that point, @lachlanglen. I've just tried to retain as much as possible from previous contributors and ensure compatibility with already deployed contracts that have the metadata.

[frol]

I was originally suggesting this NEP extension, so I, obviously support it. I would love to hear if there are other opinions from other WG members cc @fadeevab @robert-zaremba

[fadeevab]

@Canvinus Thank you for the NEP!

(+@frol +@lachlanglen)

Allow me to share my overall perception:

1. The structure may be overly flexible: all fields can be omitted.
2. version could be mandatory, e.g. version is mandatory for public crates or NPM packages.
3. What if we flatten the structure? For instance, by doing that, we can see that the contract_path can be merged into the link field, and probably tag/digest could be simplified (using a commit hash instead?).
4. One more: ☝ build_command cannot guarantee obtaining exactly the same image with the same hash and properties because of dependencies and the compiler which may change over time (at least in minor versioning).
   However, the version + CI/CD release link with the permanently stored release build guarantees that we can look into the past: proven by having a security audit of EVERY update of the $USN contract while winding down the $USN project. Because GitHub Workflow contains the commit, test results, and the release binary, the hashes were compared to what's on the blockchain.
   Nevertheless, I admit the build_command can be useful overall.

Finally, let me share my thoughts about flattening the structure as a matter of discussion:

type ContractSourceMetadata = {
  version: string, // it could be mandatory, e.g. version is mandatory for publishing Rust crates, NPM packages
  source: string|null, // it could be the full path, e.g. https://github.com/my/repo/contract/src, in this case the `contract_path` is redundant
  commit: Hash|Tag|null, // commit "bf48943..." or "v0.1.0" tag (@lachlanglen)
  build: string|null, // build command or link to CI/CD build/release
  standards: Standard[]|null,
}

[frol]

One more: ☝ build_command cannot guarantee obtaining exactly the same image

@fadeevab That is why build_details is a combination of 4: "snapshot" reference to the source code, work dir where the build has happened withing the code snapshot, build command, and the docker image reference (name [tag] + unique identifier [digest]). You may want to review this comment.

@Canvinus I am not completely sold on your naming, but I would love to hear other opinions since I am obviously biased. I tried to keep the naming generic enough to allow some other solutions but Docker for env, and other solutions besides GitHub or even git (ipfs, tarball on s3, etc).

version could be mandatory, e.g. version is mandatory for public crates or NPM packages.

That would be a breaking change for NEP-330, and I don't see a good argument for that. If someone does not want to expose the version, it is up to them. It is really designed to be flexible. It is not a requirement to have this metadata to deploy your contract on the chain, right?

[fadeevab]

@frol

One more: ☝ build_command cannot guarantee obtaining exactly the same image

@fadeevab ... and the docker image reference (name [tag] + unique identifier [digest]). You may want to review this comment.

I didn't realize that:

1. The "image" means a "docker image", not a "contract binary" in a raw form.
2. The tag is a hub.docker.com image tag, not a git tag.
3. The digest is the docker image digest.

(And right, tag/digest is just a Docker Hub way to encode an image).

Suggestions:

• EnvImage could be renamed to DockerImage.
• BuildDetails should be clarified better in the NEP.
• I still think that contract_path is unnecessary (although it's optional of course).
• A developer should be advised to commit Cargo.lock.

☝I see that the NEP is biased toward defining build environments via public docker images.
🤔However, I still can imagine not using BuildDetails and having just a link to a CI/CD public release to fit this NEP to my needs.
🙂Overall, I faced some confusion after jumping here from my bubble.

version could be mandatory, e.g. version is mandatory for public crates or NPM packages.
If someone does not want to expose the version, it is up to them.
That would be a breaking change for NEP-330

Here I saw that the NEP is on the review and my thoughts were that the NEP is not already adopted. Need to update the README's table.

It is really designed to be flexible. It is not a requirement to have this metadata to deploy your contract on the chain, right?

There is a case when the whole ContractSourceMetadata is empty.
I was also thinking about making a version mandatory to make at least 1 field mandatory. I would also require developers to define

---

The bottom line is that the NEP can be improved with more clarification and examples.

---

P.S.: On a side note, I once had in my experience that a compiler attached a timestamp into a binary, so the digest never was the same. I'm not sure it's true nowadays for cargo/rustc. Nevertheless, I'm biased toward the GitHub Workflow+Release model which has proven to be simple and effective.

[Canvinus]

@Canvinus I am not completely sold on your naming, but I would love to hear other opinions since I am obviously biased. I tried to keep the naming generic enough to allow some other solutions but Docker for env, and other solutions besides GitHub or even git (ipfs, tarball on s3, etc).

  /// Reference to a reproducible build environment, e.g. Docker image reference:
  /// "docker.io/sourcescan/cargo-near:0.6.0"
  build_environment_ref: Option<String>,

@frol, I think we should also check if a Docker image is the right one by using its digest. So, we could suggest using the image's "digest" instead of its "tag" by default. For example, [REPOSITORY]@[DIGEST] or ghcr.io/[REPOSITORY]@[DIGEST]. This method is simpler because we just need one string, and it could work for other approaches, not just Docker.

Also this won't be a problem by pulling via digest:

P.S.: On a side note, I once had in my experience that a compiler attached a timestamp into a binary, so the digest never was the same. I'm not sure it's true nowadays for cargo/rustc. Nevertheless, I'm biased toward the GitHub Workflow+Release model which has proven to be simple and effective.

[Canvinus]

@fadeevab, would having a "source_code_snapshot" string field as the source for the CI/CD release, along with a "build_environment_ref" as a string field that could point to the commit of that published release, meet your requirements?

[fadeevab]

@Canvinus I think I could use the link field and put the commit hash in the version field, so no additional fields is required. Also, commit can be found/recorded in the public release builds, e.g. https://github.com/DecentralBankDAO/usn/releases/tag/v2.3.4

[robert-zaremba]

EnvImage could be renamed to DockerImage.

Let's use Container. There can be other container images than hub.docker.com

[robert-zaremba]

I think I could use the link field and put the commit hash in the version field, so no additional fields is required.

@fadeevab I think it's good to keep both version and field. If a code is uploaded without a version control (which should be a very rare case), then version would just be human friendly semantic number (eg v1.0.1) and must be the same as package / crate version.

[robert-zaremba]

Recommendation

This pull request is an improvement to the NEP-330 v1.1.0 and I lean to approve it once the small concerns listed below are resolved.

Concerns

• let's be more specific about optional and required fields. For example, if using a version control system (git...) then version must be required.
• be more specific about version and link - I suggest to add more examples (made few suggestions).
• solve naming. Suggestions: env_image -> container, build_details -> build_info

[fadeevab]

Recommendation

@near/wg-contract-standards As the Contract Standards WG member, I lean towards approving this NEP extension after resolving the concerns in the table below.

Benefits

The standard extension effectively adds a build_info to the contract metadata in a backward-compatible manner.

type ContractSourceMetadata = {
    version: string|null,
    link: string|null,
    standards: Standard[]|null,
+    build_info: BuildInfo|null, // optional, details required for contract WASM reproducibility.
}

The NEP provides examples of how the fields of BuildInfo can be customized and how build reproducibility can be achieved.

Concerns

#	Concern	Resolution	Status
1	Considerations about BuildInfo structure complexity	Strusture was flattened and simplified	Resolved
2	How to properly use the NEP	More details and examples added	Resolved
3	Build reproducibility via Docker images	Details about how to use build_environment and build_command	Resolved
4	Concerns about 3rd party dependencies which affect reproducibility	Suggest committing Cargo.lock	Resolved[1]
5	Using public CI/CD releases instead of Docker environment as an option	Using build_environment link to a custom env	Won't be done

[1]: Should be completed within the issue near/cargo-near-new-project-template#9

[robert-zaremba]

Recommendation

@near/wg-contract-standards As the Contract Standards WG member, I lean towards approving this NEP extension after resolving the Cargo.lock suggestion form @fadeevab

Concerns

#	Concern	Resolution	Status
1	be more specific about optional and required fields	spec updated	Resolved
2	be more specific about version and link field	link is an URL to version of repository when available	Resolved
3	consider changing field names	discussed and renamed	Resolved

[victorchimakanu]

NEP Status (Updated by NEP Moderators)

Status: Approved

SME reviews:

• Contract Standards SME @robert-zaremba: NEP-330 extension: Build details extension #533 (review)
• Contract Standards SME @fadeevab: NEP-330 extension: Build details extension #533 (comment)

Contract Standards Work Group voting indications (❔ | 👍 | 👎 ):

• 👍 @frol NEP-330 extension: Build details extension #533 (comment)
• 👍 @robert-zaremba NEP-330 extension: Build details extension #533 (comment)
• 👍 @fadeevab NEP-330 extension: Build details extension #533 (comment)

[fadeevab]

@victorchimakanu we still have unfinished Cargo.lock issue #533 (comment) @Canvinus what's the status?..

[Canvinus]

@fadeevab, I talked with @frol about it, and they will update cargo-near-new-project-template to include Cargo.lock. As soon as that happens, I will include the latest commit of cargo-near-new-project-template as an example.

[fadeevab]

@Canvinus @frol Agree! I created a tracking issue near/cargo-near-new-project-template#9 (don't forget it!), let's move forward with this NEP

[victorchimakanu]

As the moderator, I would like to thank the author @Canvinus and @alexzinin for submitting this NEP, and the NEAR contracts standards work group members for their reviews. Based on the voting indications above, it seems like this proposal is close to a decision. We are therefore scheduling the Fifth Contract standards Work group meeting next week, where this NEP can enter the final voting stage. Anyone can discuss the technical details by adding your comments to this discussion. And join the call to learn about the final decision and how to get more involved.”

Meeting Info

NEP Discussion: NEP-330
📅 Date: Thursday April 18th 2024 5PM UTC
✏️ Add to Calendar

[fadeevab]

@Canvinus It makes sense to fix a linter error
Wrong url: https://rust-random.github.io/rand/rand/distributions/weighted/alias_method/struct.WeightedIndex.html
Good url: https://rust-random.github.io/rand/rand/distributions/struct.WeightedIndex.html

[Canvinus]

@Canvinus It makes sense to fix a linter error Wrong url: https://rust-random.github.io/rand/rand/distributions/weighted/alias_method/struct.WeightedIndex.html Good url: https://rust-random.github.io/rand/rand/distributions/struct.WeightedIndex.html

Looks like it still has bunch of linter errors)

[fadeevab]

@Canvinus It makes sense to fix a linter error Wrong url: https://rust-random.github.io/rand/rand/distributions/weighted/alias_method/struct.WeightedIndex.html Good url: https://rust-random.github.io/rand/rand/distributions/struct.WeightedIndex.html

Looks like it still has bunch of linter errors)

Oh, right :) Usually, a markdown formatter in VS Code fixes 99% of those issues for me, e.g. placing spaces around headers. I guess, you should try.

[frol]

Thank you to everyone who attended the Contract Standards Working Group meeting today! The working group members reviewed the NEP and reached the following consensus:

Status: Approved (Meeting Recording: https://youtu.be/JQu8tObzhlM)

• 👍 @frol NEP-330 extension: Build details extension #533 (comment)
• 👍 @robert-zaremba NEP-330 extension: Build details extension #533 (comment)
• 👍 @fadeevab NEP-330 extension: Build details extension #533 (comment)

@Canvinus Thank you for championing the extension to NEP-330

@fadeevab @robert-zaremba Thank you for all the review!

Next steps:

• @frol should merge the NEP PR
• Adopt the implementations on the cargo-near, SourceScan, and beyond (e.g. NearBlocks)