Skip to content

Feat/contracts with original requests - #4245

Open
carpawell wants to merge 3 commits into
masterfrom
feat/contracts-with-original-requests
Open

carpawell wants to merge 3 commits into
masterfrom
feat/contracts-with-original-requests

Conversation

@carpawell

Copy link
Copy Markdown
Member

No description provided.

@carpawell carpawell self-assigned this Oct 21, 2026
@carpawell
carpawell force-pushed the feat/contracts-with-original-requests branch from 261b014 to 11dbdf2 Compare October 22, 2026 17:34
@carpawell
carpawell marked this pull request as ready for review October 22, 2026 17:39
@roman-khimov
roman-khimov requested a balanced review from Copilot October 22, 2026 20:08

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Unresolved authorization gaps and signature compatibility regressions can approve unauthorized changes or reject legitimate requests.

7 open issues
What changed in this PR

This PR carries original container API requests through contract calls so Inner Ring processors can validate them before approving container creation, deletion, and eACL updates.

Changes:

  • Forward serialized requests from the container service through blockchain clients.
  • Parse original-request contract arguments and add Inner Ring validation.
  • Update the contract dependency, test doubles, and changelog.
File Description
pkg/​services/​container/​service_internal_test.go Update mock contract signatures.
pkg/​services/​container/​server.go Serialize and forward original API requests.
pkg/​services/​container/​server_test.go Adapt test contracts to the new parameters.
pkg/​morph/​event/​container/​notary_requests.go Parse legacy and original-request call formats.
pkg/​morph/​contracts/​methods.go Add original-request fields to contract parameters.
pkg/​morph/​client/​container/​put.go Support creation calls carrying original requests.
pkg/​morph/​client/​container/​eacl_set.go Support eACL calls carrying original requests.
pkg/​morph/​client/​container/​delete.go Support deletion calls carrying original requests.
pkg/​innerring/​processors/​container/​process_eacl.go Add original-request validation for eACL updates.
pkg/​innerring/​processors/​container/​process_container.go Add shared request validation for container operations.
go.sum Update contract dependency checksums.
go.mod Advance the contract dependency.
cmd/​neofs-node/​container.go Forward original requests to blockchain clients.
CHANGELOG.md Document Inner Ring request validation.

Review effort: Balanced

Comment thread pkg/innerring/processors/container/process_container.go Outdated
Comment thread pkg/innerring/processors/container/process_container.go Outdated
signedReq, _ := protoencoding.EncodeRequest(nil, reqBody, reqMetaHeader)
sig := reqVerifHeader.GetRequestSignature()

err := cp.verifySignature(signatureVerificationData{

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

To be fixed separately.

Comment thread pkg/innerring/processors/container/process_eacl.go
Comment thread pkg/innerring/processors/container/process_container.go Outdated
Comment thread pkg/innerring/processors/container/process_container.go Outdated
Comment thread pkg/innerring/processors/container/process_container.go Outdated
carpawell added a commit to nspcc-dev/neofs-sdk-go that referenced this pull request Oct 23, 2026
These cases are special, they require any signature routine to be done by
container owner, that is why it is passed as a func param. In particular, it
allows nspcc-dev/neofs-node#4245 exchanging container,
containerID, and container eACL signatures with the whole request signatures.
Refs nspcc-dev/neofs-api#446.

Signed-off-by: Pavel Karpy <carpawell@nspcc.io>
@carpawell
carpawell force-pushed the feat/contracts-with-original-requests branch from 11dbdf2 to 1c3bac2 Compare October 23, 2026 16:47
@codecov

codecov Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 9.05350% with 221 lines in your changes missing coverage. Please review.
✅ Project coverage is 32.26%. Comparing base (2c7b39e) to head (b6eb42e).
⚠️ Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
...nnerring/processors/container/process_container.go 0.00% 97 Missing ⚠️
pkg/morph/event/container/notary_requests.go 0.00% 75 Missing ⚠️
pkg/innerring/processors/container/process_eacl.go 0.00% 15 Missing ⚠️
pkg/morph/client/container/put.go 0.00% 12 Missing ⚠️
pkg/morph/client/container/delete.go 0.00% 7 Missing ⚠️
pkg/morph/client/container/eacl_set.go 0.00% 7 Missing ⚠️
pkg/services/container/server.go 81.48% 5 Missing ⚠️
cmd/neofs-node/container.go 0.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4245      +/-   ##
==========================================
- Coverage   32.35%   32.26%   -0.10%     
==========================================
  Files         673      673              
  Lines       41430    41610     +180     
==========================================
+ Hits        13406    13424      +18     
- Misses      28024    28186     +162     

☔ View full report in Codecov by Harness.
📢 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 added blocked Can't be done because of something and removed blocked Can't be done because of something labels Oct 23, 2026
It now has original API request for calls that are initiated by external users.

Signed-off-by: Pavel Karpy <carpawell@nspcc.io>
It now has container RPC signed by container owner (or by an authorized key) and
allows IRs checking original requests by its signatures, not by request fields.
Refs nspcc-dev/neofs-api#446,
nspcc-dev/neofs-sdk-go#898.

Signed-off-by: Pavel Karpy <carpawell@nspcc.io>
Implementation of nspcc-dev/neofs-contract#637.

Signed-off-by: Pavel Karpy <carpawell@nspcc.io>
@carpawell
carpawell force-pushed the feat/contracts-with-original-requests branch from 1c3bac2 to b6eb42e Compare October 26, 2026 16:54

@roman-khimov roman-khimov left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Rebase, please.

withOptionalEacl bool
err error
)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👀

signedReq, _ := protoencoding.EncodeRequest(nil, reqBody, reqMetaHeader)
sig := reqVerifHeader.GetRequestSignature()

err := cp.verifySignature(signatureVerificationData{

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

To be fixed separately.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

}

err = cp.checkSetEACL(*req.EACLTable, table, id, cnr)
err = cp.checkSetEACL(*req.EACLTable, table, id, cnr, req.OriginalRequest == nil)
Comment on lines +378 to +379
if !multipleReqSignatures(reqMetaHeader) {
return nil
return fmt.Errorf("container ID in the original request does not match the contract one")
}

return cp.checkOriginalRequest(cnr, cID, session.VerbContainerSetEACL, sessionv2.VerbContainerSetEACL, apiReq.GetBody(), apiReq.GetMetaHeader(), apiReq.GetVerifyHeader())
Comment on lines +398 to +402
verifData.signedData = signedData
verifData.verifScript = sig.GetKey()
verifData.invocScript = sig.GetSign()

err = cp.verifySignature(verifData)

This branch has not been deployed

No deployments
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