Skip to content

Support structured containers#3670

Merged
roman-khimov merged 11 commits intomasterfrom
support-structured-containers
Dec 2, 2025
Merged

Support structured containers#3670
roman-khimov merged 11 commits intomasterfrom
support-structured-containers

Conversation

@cthulhu-rider
Copy link
Contributor

@cthulhu-rider cthulhu-rider commented Nov 6, 2025

@codecov
Copy link

codecov bot commented Nov 6, 2025

Codecov Report

❌ Patch coverage is 0% with 264 lines in your changes missing coverage. Please review.
✅ Project coverage is 27.22%. Comparing base (79e4ff0) to head (c7b5407).
⚠️ Report is 12 commits behind head on master.

Files with missing lines Patch % Lines
pkg/morph/client/container/models.go 0.00% 60 Missing ⚠️
...nnerring/processors/container/process_container.go 0.00% 53 Missing ⚠️
pkg/innerring/processors/container/processor.go 0.00% 44 Missing ⚠️
pkg/morph/event/container/notary_requests.go 0.00% 34 Missing ⚠️
pkg/morph/client/container/put.go 0.00% 20 Missing ⚠️
pkg/morph/client/container/get.go 0.00% 17 Missing ⚠️
pkg/morph/client/notary.go 0.00% 16 Missing ⚠️
pkg/morph/client/container/util.go 0.00% 7 Missing ⚠️
pkg/innerring/processors/container/handlers.go 0.00% 6 Missing ⚠️
pkg/innerring/innerring.go 0.00% 5 Missing ⚠️
... and 1 more
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3670      +/-   ##
==========================================
- Coverage   27.32%   27.22%   -0.10%     
==========================================
  Files         657      659       +2     
  Lines       41827    42008     +181     
==========================================
+ Hits        11429    11437       +8     
- Misses      29335    29510     +175     
+ Partials     1063     1061       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@cthulhu-rider cthulhu-rider force-pushed the support-structured-containers branch 3 times, most recently from 03db265 to 36e3eae Compare November 18, 2025 11:47
@cthulhu-rider cthulhu-rider force-pushed the support-structured-containers branch 6 times, most recently from f6c53ef to fb20039 Compare November 25, 2025 09:56
@cthulhu-rider cthulhu-rider marked this pull request as ready for review November 25, 2025 10:04
old := err != nil && isMethodNotFoundError(err, method)
if old {
method = getMethod
if err != nil && isMethodNotFoundError(err, method) {
Copy link
Member

Choose a reason for hiding this comment

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

imo, fallback to old methods while we're sure it exists (our networks are always updated in sync with SNs) may hide unexpected errors. i would always call new methods in the same release IR is expected with new contracts

Copy link
Member

@roman-khimov roman-khimov Nov 28, 2025

Choose a reason for hiding this comment

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

While currently all nodes are controlled by NeoSPCC everywhere it might not be the case in future. In which case we can get both "new IR/old SN" (which we do get anyway during the upgrade and we usually keep enough compatibility for) and "old IR/new SN" combos in case someone decides to upgrade an SN before we're to update IR. This compatibility code can be very much helpful in this case.

go s.mainnetListener.ListenWithError(ctx, mainnnetErr) // listen for neo:mainnet events

go func() {
if err := s.containerProcessor.AddContainerStructs(ctx); err != nil {
Copy link
Member

Choose a reason for hiding this comment

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

so the plan is to always send this tx at startup and sign every NR we receive from the chain?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

true

Copy link
Member

@carpawell carpawell Dec 2, 2025

Choose a reason for hiding this comment

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

so it will be sent at every restart of this exact version of IR?

Copy link
Member

Choose a reason for hiding this comment

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

but mb that is not a problem

Brings nspcc-dev/neofs-contract#534.

Signed-off-by: Leonard Lyubich <leonard@morphbits.io>
@cthulhu-rider cthulhu-rider force-pushed the support-structured-containers branch 2 times, most recently from d96ad08 to 3109ffb Compare December 1, 2025 08:53
}

func (cp *Processor) approvePutContainer(ctx *putContainerContext, id cid.ID) {
func (cp *Processor) approvePutContainer(mainTx transaction.Transaction, cnr containerSDK.Container, id cid.ID) {
Copy link
Member

Choose a reason for hiding this comment

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

transaction.Transaction is almost never passed by value. Not that it's an error, but likely an inefficiency of some kind (unless there are some modifier of course, but I hope we already solved those).

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 use a simple pattern that if there is no change - there is no point in pointer

Copy link
Member

Choose a reason for hiding this comment

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

This easily leads to excessive copying unless compiler has enough reasons to optimize it out.

If new method is missing, SN falls back to the old ones.

Signed-off-by: Leonard Lyubich <leonard@morphbits.io>
It's not needed in this case.

Signed-off-by: Leonard Lyubich <leonard@morphbits.io>
This will make it easier to support nspcc-dev/neofs-contract#534.

Signed-off-by: Leonard Lyubich <leonard@morphbits.io>
Will become available with nspcc-dev/neofs-contract#534
upgrade. If new method is missing, SN falls back to old methods.

Signed-off-by: Leonard Lyubich <leonard@morphbits.io>
@cthulhu-rider cthulhu-rider force-pushed the support-structured-containers branch from 3109ffb to ab46198 Compare December 2, 2025 05:01
It makes no sense to keep domain as struct field.

Signed-off-by: Leonard Lyubich <leonard@morphbits.io>
Signed-off-by: Leonard Lyubich <leonard@morphbits.io>
Passing explicit parameters is easier to understand.

Signed-off-by: Leonard Lyubich <leonard@morphbits.io>
Signed-off-by: Leonard Lyubich <leonard@morphbits.io>
Will become available with nspcc-dev/neofs-contract#534
upgrade.

Signed-off-by: Leonard Lyubich <leonard@morphbits.io>
@cthulhu-rider cthulhu-rider force-pushed the support-structured-containers branch from ab46198 to 88fe603 Compare December 2, 2025 05:03
@roman-khimov
Copy link
Member

Lint/tests

Within nspcc-dev/neofs-contract#449, there is
a need to store containers as VM structs. Since
nspcc-dev/neofs-contract#534, struct items are
stored for new containers. This also needs to be done for containers
created before the upgrade.

Since doing this in a contract update transaction turned out to be too
GAS-intensive, this implements structuring through the IR background
process. Specialized method is called iteratively doing the same thing,
but for several containers at a time.

Signed-off-by: Leonard Lyubich <leonard@morphbits.io>
@cthulhu-rider cthulhu-rider force-pushed the support-structured-containers branch from 88fe603 to c7b5407 Compare December 2, 2025 06:46
@cthulhu-rider
Copy link
Contributor Author

fixed

@roman-khimov roman-khimov merged commit 1bda439 into master Dec 2, 2025
20 of 22 checks passed
@roman-khimov roman-khimov deleted the support-structured-containers branch December 2, 2025 12:25
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.

3 participants