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

Provide embedded compiled NeoFS contracts #2391

Merged

Conversation

cthulhu-rider
Copy link
Contributor

@cthulhu-rider cthulhu-rider commented Jun 18, 2023

isolated changes from #2301. Starting from this, I'll try introduce changes partially to lighten review process

note that these changes do not yet present the script for compiling and copying contract files into contracts dir, suggestions are welcome

in contrast to the proposed option for naming files in #2195, i decided not to name files with serial numbers because to me it's clearer to see what contract is represented by particular files. Anyway, this pattern can be changed at any time

@codecov
Copy link

codecov bot commented Jun 18, 2023

Codecov Report

Merging #2391 (8d48fb6) into master (93733ff) will increase coverage by 0.10%.
The diff coverage is 82.25%.

❗ Current head 8d48fb6 differs from pull request most recent head 54874e4. Consider uploading reports for the commit 54874e4 to get more accurate results

@@            Coverage Diff             @@
##           master    #2391      +/-   ##
==========================================
+ Coverage   30.03%   30.14%   +0.10%     
==========================================
  Files         398      399       +1     
  Lines       29806    29868      +62     
==========================================
+ Hits         8953     9004      +51     
- Misses      20128    20134       +6     
- Partials      725      730       +5     
Impacted Files Coverage Δ
contracts/contracts.go 82.25% <82.25%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@roman-khimov
Copy link
Member

You can name them like 01-alphabet, 20-netmap, etc. But it's important since you can easily see the deployment sequence, you can make the code more generic (especially with reduced number of deployment parameters) and you can more easily stuff some new contract into the mix.

@roman-khimov
Copy link
Member

The only reason for not doing so, btw, is storing contracts in neofs-contract and getting them right from that repo. This in fact can be done with a simple wrapper that would make a proper Go package out of these files.

@cthulhu-rider cthulhu-rider force-pushed the feature/embedded-contracts branch 3 times, most recently from b88112b to 4918ecf Compare June 20, 2023 09:33
@cthulhu-rider
Copy link
Contributor Author

@roman-khimov moved to NN.nef + NN.manifest.json naming pattern, check

for now added NNS contract only just to prevent go:embed directive failure

There is a need to embed Neo contracts' executables into NeoFS Inner
Ring application. To do this, `contracts` directory is created. The dir
contains compiled contracts (per-contract NEF and manifest). Create
eponymous Go package that provides embedded `fs.FS` with the contracts.
Add `Read` function which reads, decodes and validates all
numerically-sored contracts from files and returns ready-to-go data for
deployment.

Refs nspcc-dev#2195.

Signed-off-by: Leonard Lyubich <leonard@morphbits.io>
@roman-khimov roman-khimov merged commit cfb7822 into nspcc-dev:master Jun 20, 2023
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.

2 participants