Skip to content

Feat/unpaid containers handling#3650

Merged
roman-khimov merged 12 commits intomasterfrom
feat/unpaid-containers-handling
Nov 21, 2025
Merged

Feat/unpaid containers handling#3650
roman-khimov merged 12 commits intomasterfrom
feat/unpaid-containers-handling

Conversation

@carpawell
Copy link
Member

No description provided.

@carpawell carpawell added the blocked Can't be done because of something label Oct 24, 2025
@carpawell
Copy link
Member Author

Still not sure about dropping container immediately after it is unpaid. How long to wait?

@carpawell carpawell force-pushed the feat/unpaid-containers-handling branch from a143206 to c0f3e98 Compare October 24, 2025 00:03
@codecov
Copy link

codecov bot commented Oct 24, 2025

Codecov Report

❌ Patch coverage is 19.07216% with 157 lines in your changes missing coverage. Please review.
✅ Project coverage is 27.35%. Comparing base (da778da) to head (5056ef6).
⚠️ Report is 16 commits behind head on master.

Files with missing lines Patch % Lines
cmd/neofs-node/container.go 0.00% 55 Missing ⚠️
pkg/morph/client/notary.go 0.00% 28 Missing ⚠️
pkg/morph/client/balance/payment.go 0.00% 24 Missing ⚠️
pkg/morph/event/balance/payment.go 0.00% 15 Missing ⚠️
pkg/innerring/processors/settlement/calls.go 0.00% 8 Missing ⚠️
cmd/neofs-node/config.go 0.00% 6 Missing ⚠️
pkg/services/object/put/streamer.go 37.50% 3 Missing and 2 partials ⚠️
internal/testutil/log.go 0.00% 4 Missing ⚠️
pkg/morph/client/static.go 0.00% 4 Missing ⚠️
pkg/local_object_storage/shard/gc.go 85.71% 2 Missing and 1 partial ⚠️
... and 5 more
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3650      +/-   ##
==========================================
+ Coverage   27.34%   27.35%   +0.01%     
==========================================
  Files         658      659       +1     
  Lines       41713    41807      +94     
==========================================
+ Hits        11407    11438      +31     
- Misses      29246    29309      +63     
  Partials     1060     1060              

☔ 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.

@carpawell carpawell force-pushed the feat/unpaid-containers-handling branch from c0f3e98 to 65295a5 Compare October 24, 2025 01:23
@carpawell carpawell force-pushed the feat/unpaid-containers-handling branch from 65295a5 to b2a9c45 Compare November 18, 2025 15:02
@carpawell carpawell removed the blocked Can't be done because of something label Nov 18, 2025
@carpawell carpawell force-pushed the feat/unpaid-containers-handling branch 3 times, most recently from ee1efdf to 20f8020 Compare November 19, 2025 21:03
@carpawell carpawell marked this pull request as ready for review November 19, 2025 21:05
@carpawell carpawell force-pushed the feat/unpaid-containers-handling branch 2 times, most recently from c226022 to 698ca3c Compare November 20, 2025 09:01

p.statuses[cID] = int64(ev.Epoch)
} else {
l.Debug("container status has changed to paid",
Copy link
Member

Choose a reason for hiding this comment

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

Info?

Consider the following scenario as well:

  • container gets "unpaid" status
  • all data is deleted from nodes
  • its size is zero, so there are no settlements to make
  • but its status is still "unpaid" even if owner has some balance

Can this be a problem?

Copy link
Member Author

@carpawell carpawell Nov 20, 2025

Choose a reason for hiding this comment

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

Info?

hm, i considered "soon be deleted" as an important log, while "everything now ok" as a debug. but dont mind INFO

  • its size is zero, so there are no settlements to make

i guess, there we should make a zero report in the next epoch. i can suggest dropping everything about unpaid container, but keep its (0,0) statistics in metabase, then this will be reported, and its status will be changed back. what do you think? any immediately forced report may be the 4-th (prohibited) in this epoch, and we cannot be sure it will be successful

Copy link
Member

Choose a reason for hiding this comment

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

Let's move this into a separate issue.

// nodes that maintain objects inside the container. Always sends a notary
// request with Alphabet multi-signature. Always awaits transaction inclusion.
func (c *Client) SettleContainerPayment(cID cid.ID) error {
func (c *Client) SettleContainerPayment(epoch uint64, cID cid.ID) error {
Copy link
Member

Choose a reason for hiding this comment

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

But ultimately it's controlled by balance contract, right? I mean double-payment prevention.

Copy link
Member Author

Choose a reason for hiding this comment

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

what i meant in this commit: if epoch duration is short enough, it was impossible to pay for a container twice in two or more close epochs, cause tx's hash was the same. this could be seen in our tests. but if you mean that the contract should not allow paying twice per epoch, i do not see what would stop it. is it a problem?

Copy link
Member

Choose a reason for hiding this comment

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

It sure is, contracts must ensure payment correctness. Submit an issue for contracts, but it's not top priority now.

@carpawell carpawell force-pushed the feat/unpaid-containers-handling branch from 698ca3c to 90a97c8 Compare November 20, 2025 17:59
Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
Implementation of nspcc-dev/neofs-contract#531.

Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
Calculate nothing on the Alphabet side, only send payment transactions for
containers. Closes #3357.

Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
Closes #774.

Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
Do not log if nothing to report; report total number of containers found; fix
success number log; do not confuse reader with a general logs in singular form.

Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
This allows debugging problems much easier compared to hiding errors.

Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
Use epoch as a nonce to be able to transfer the same amount of tokens to
storage nodes more than once.

Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
Naming is the same as in `require` package.

Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
No dropping yet for the first feature release.

Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
@carpawell carpawell force-pushed the feat/unpaid-containers-handling branch from 90a97c8 to 5056ef6 Compare November 20, 2025 18:02
@roman-khimov roman-khimov merged commit e129c34 into master Nov 21, 2025
20 of 22 checks passed
@roman-khimov roman-khimov deleted the feat/unpaid-containers-handling branch November 21, 2025 09:10
carpawell added a commit that referenced this pull request Mar 10, 2026
Not only a warning, but a full cleanup. Refs #3650. Closes #3691.

Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
carpawell added a commit that referenced this pull request Mar 10, 2026
Not only a warning, but a full cleanup. Refs #3650. Closes #3691.

Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
carpawell added a commit that referenced this pull request Mar 11, 2026
… enabled

Refs #3650.

Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
carpawell added a commit that referenced this pull request Mar 11, 2026
Not only a warning, but a full cleanup. Refs #3650. Closes #3691.

Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
carpawell added a commit that referenced this pull request Mar 11, 2026
… enabled

Refs #3650.

Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
roman-khimov added a commit that referenced this pull request Mar 11, 2026
Not only a warning, but a full cleanup. Refs #3650. Closes #3691.
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