Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR removes support for tombstone objects with API version < 2.18, which previously stored deleted object references in the payload. The new implementation only supports the modern tombstone format (API v2.18+) that stores a single deleted object reference in the header via the AssociatedObject field.
Key Changes:
- Removed
VerifyTombmethod that validated old-format tombstones with multiple members - Consolidated validation logic for Lock and Tombstone objects using parameterized error messages
- Removed test cases for deprecated tombstone formats
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pkg/services/object/tombstone/verify.go | Removed VerifyTomb method and associated concurrency handling; kept only VerifyTombStoneWithoutPayload for API v2.18+ |
| pkg/services/object/tombstone/verify_test.go | Removed tests for old tombstone formats; updated remaining tests to use new API exclusively; removed unused helper functions |
| pkg/core/object/fmt.go | Removed VerifyTomb from TombVerifier interface; consolidated Lock/Tombstone validation with stricter checks; simplified ValidateContent to reject pre-2.18 system objects |
| pkg/core/object/fmt_test.go | Removed test case for old tombstone content validation with payload |
| pkg/core/object/ec_test.go | Updated tombstone EC test to properly configure v2.18 tombstone with header-based target |
| pkg/services/object/put/service_test.go | Added explicit SetType(object.TypeRegular) calls to test objects to satisfy stricter validation |
| CHANGELOG.md | Added entry documenting that tombstone objects with API < 2.18 are no longer accepted |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #3731 +/- ##
==========================================
- Coverage 26.89% 26.84% -0.05%
==========================================
Files 658 658
Lines 41861 41814 -47
==========================================
- Hits 11258 11226 -32
+ Misses 29566 29552 -14
+ Partials 1037 1036 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
2f2214c to
b95ea25
Compare
Member
Author
|
All tests (running now): https://github.com/nspcc-dev/neofs-node/actions/runs/20104857374 |
b95ea25 to
949d434
Compare
Member
Author
Member
Author
carpawell
reviewed
Dec 11, 2025
That's where it should be, Validate() checks headers and expiration should be set immediately. This avoids double-checking and also allows to catch problems faster. Object PUT test actually reveals that, by default test object is a tombstone and it was missing expiration attribute. Signed-off-by: Roman Khimov <roman@nspcc.ru>
Now they're essentially the same, except for one thing. Drop duplicating checks from VerifyTombStoneWithoutPayload as well. Signed-off-by: Roman Khimov <roman@nspcc.ru>
"Unprepared" objects are often missing versions, but the don't have payloads normally anyway and associated attribute must be set already, otherwise it'll be invalid later. In some ways attribute check makes version check somewhat excessive, but it's left for now. Signed-off-by: Roman Khimov <roman@nspcc.ru>
It's always empty since 45150c9, this data is unused and many more functions can be simplified now because of that. Signed-off-by: Roman Khimov <roman@nspcc.ru>
Originally verifier only cared about relations, but it also has the header of the target object, so a type check costs nothing here while it's very much useful. TS can't target another TS and LOCK by protocol and LINKs can't be deleted as well since user needs to delete the parent object. I believe this even fixes #3498 although at a bit different (pre-engine) level, but engine can be somewhat more dumb. Signed-off-by: Roman Khimov <roman@nspcc.ru>
949d434 to
d068a59
Compare
carpawell
approved these changes
Dec 11, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A part of #3476.