Feat/push object meta to contracts #3044

carpawell · 2024-12-10T01:04:11Z

No description provided.

codecov · 2024-12-10T21:15:31Z

Codecov Report

Attention: Patch coverage is 10.53922% with 365 lines in your changes missing coverage. Please review.

Project coverage is 22.36%. Comparing base (4f4840c) to head (ae6e37d).
Report is 26 commits behind head on master.

Files with missing lines	Patch %	Lines
pkg/services/object/put/distributed.go	0.00%	77 Missing ⚠️
pkg/morph/client/container/nodes.go	0.00%	47 Missing ⚠️
pkg/morph/client/notary.go	0.00%	46 Missing ⚠️
pkg/morph/event/container/put_notary.go	0.00%	45 Missing ⚠️
pkg/innerring/processors/netmap/process_epoch.go	0.00%	38 Missing ⚠️
...nnerring/processors/container/process_container.go	0.00%	28 Missing ⚠️
pkg/morph/client/container/meta.go	0.00%	15 Missing ⚠️
pkg/morph/client/container/put.go	0.00%	11 Missing ⚠️
pkg/morph/client/static.go	0.00%	8 Missing ⚠️
pkg/morph/event/opcodes.go	0.00%	8 Missing ⚠️
... and 15 more

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3044      +/-   ##
==========================================
- Coverage   22.41%   22.36%   -0.06%     
==========================================
  Files         791      793       +2     
  Lines       58405    58698     +293     
==========================================
+ Hits        13093    13127      +34     
- Misses      44416    44669     +253     
- Partials      896      902       +6

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

carpawell · 2024-12-11T17:07:02Z

pkg/innerring/processors/netmap/process_epoch.go

+		return fmt.Errorf("can't get containers list: %w", err)
+	}
+
+	for _, cID := range cids {


@roman-khimov, i think this is the only thing that we may discuss: IMO if netmap is changed, many containers change their placement and it is easier to recalculate them all once in a single epoch (and not touch them for many-many epochs) than to cache the whole map from the previous epoch OR to fetch it, calculate and compare every vector in every container to see if anything has changed

Works for now.

carpawell · 2024-12-11T17:07:10Z

Need some decisions there. In any case, will not be hard to fix. Overall it is ready.

cthulhu-rider

sequence of commits is inconvenient for review. Some subsequent commits answer questions about the previous ones, and sometimes it seems they even fix them. And looks like the whole desired behavior achieved only by having them all at once

anyway, i'll leave all the comments accumulated by the middle just in case

cthulhu-rider · 2024-12-12T10:48:45Z

pkg/morph/client/container/nodes.go

+// AddNextEpochNodes registers public keys as a container's placement vector
+// with specified index. Registration must be finished with final
+// [Client.CommitContainerListUpdate] call. Always sends a notary request with
+// Alphabet multi-signature.


request to sign by the Alphabet to be more precise

and is always signed by one of the Alphabet node in fact. i added this like because we have AsAlphabet and smth like this option for the client. this line highlights that it is always sent by the alphabet node, SN and any other client should not so this

cthulhu-rider · 2024-12-12T10:50:58Z

pkg/morph/client/container/nodes.go

+)
+
+// AddNextEpochNodes registers public keys as a container's placement vector
+// with specified index. Registration must be finished with final


this registers, and then 2nd statetement follows. To me, this means 'call CommitContainerListUpdate right after this method'. I'd paraphrase

i can suggest "registers candidates". you mean it is not clear that it is possible to call it more that once before Submit...?

cthulhu-rider · 2024-12-12T10:53:33Z

pkg/morph/client/container/nodes.go

+// with specified index. Registration must be finished with final
+// [Client.CommitContainerListUpdate] call. Always sends a notary request with
+// Alphabet multi-signature.
+func (c *Client) AddNextEpochNodes(cid []byte, placementIndex int, nodesKeys [][]byte) error {


why placement index is not uint8 as in https://github.com/nspcc-dev/neofs-contract/blob/36dd7a25087993a6b71011056f1c1d2291d4efd4/contracts/container/contract.go#L575?

no specific idea. since it is a real go code, i wanted indexes to be int while REPs in our SDK are uint32. i wanted it to be something special but there are no types in neo VM. i even had to change something on the contract side just because it did not compile
about this thread: it was not a mistake, made intentionally, if you do not agree, ping me one more time and i will change

cthulhu-rider · 2024-12-12T10:55:22Z

pkg/morph/client/container/nodes.go

+// [Client.CommitContainerListUpdate] call. Always sends a notary request with
+// Alphabet multi-signature.
+func (c *Client) AddNextEpochNodes(cid []byte, placementIndex int, nodesKeys [][]byte) error {
+	if len(cid) == 0 || len(nodesKeys) == 0 {


not rly useful check for cid, better to accept cid.ID

cthulhu-rider · 2024-12-12T10:56:49Z

pkg/morph/client/container/nodes.go

+// CommitContainerListUpdate finishes container placement updates for the current
+// epoch made by former [Client.AddNextEpochNodes] calls. Always sends a notary
+// request with Alphabet multi-signature.
+func (c *Client) CommitContainerListUpdate(cid []byte, replicas []uint32) error {


and here it's uint32 instead of uint8

cthulhu-rider · 2024-12-12T11:17:29Z

pkg/services/object/put/distributed.go

@@ -10,6 +10,7 @@ import (

 	"github.com/nspcc-dev/neofs-node/pkg/core/client"
 	"github.com/nspcc-dev/neofs-node/pkg/core/object"
+	chaintcontainer "github.com/nspcc-dev/neofs-node/pkg/morph/client/container"


cthulhu-rider · 2024-12-12T11:20:34Z

pkg/services/object/put/distributed.go

+	}
+
+	if t.localNodeInContainer {
+		t.metaMtx.RLock()


there is no more concurrent access at this point, isn't it?

that is right but still, mutex saves exactly this field so lets read it safely too, if you don't mind. mb, it will save us in future changes

cthulhu-rider · 2024-12-12T11:21:00Z

pkg/services/object/put/distributed.go

+
+		err = t.cnrClient.SubmitObjectPut(t.objSharedMeta, t.collectedSignatures)
+		if err != nil {
+			t.placementIterator.log.Info("submitting meta information", zap.Stringer("oid", id), zap.Error(err))


failed to submit

cthulhu-rider · 2024-12-12T11:25:12Z

pkg/services/object/put/distributed.go

@@ -154,7 +155,7 @@ func (t *distributedTarget) Close() (oid.ID, error) {
 		return oid.ID{}, err
 	}

-	if t.localNodeInContainer {
+	if t.localNodeInContainer && t.metainfoConsistencyAttr != "" {


could u pls remind is t.metainfoConsistencyAttr != "" == enabled meta-on-chain?

in practice yes. but non-zero also varies so i added a switch later

cthulhu-rider · 2024-12-12T11:25:52Z

pkg/services/object/put/distributed.go

@@ -33,7 +33,8 @@ type distributedTarget struct {
 	objMeta            object.ContentMeta
 	networkMagicNumber uint32

-	cnrClient *chaintcontainer.Client
+	cnrClient               *chaintcontainer.Client
+	metainfoConsistencyAttr string


now it's handled as boolean, will behavior ever depend on the particular value?

yes, in the latter commit. hm, i see, lets squash some commits

carpawell · 2024-12-12T15:31:06Z

sequence of commits is inconvenient for review

IMO, it is the opposite, i tired to highlight step by step what should be changed to be possible it to work. Squashed few commits not to repeat some things

And looks like the whole desired behavior achieved only by having them all at once

Some extend morph client without behavior changes. Some prepare IR. Some prepare SN.

pkg/innerring/processors/container/process_container.go

roman-khimov · 2024-12-15T13:42:45Z

pkg/innerring/processors/netmap/cleanup_table.go

@@ -64,6 +68,14 @@ func (c *cleanupTable) update(snapshot netmap.NetMap, now uint64) {
 	}

 	c.lastAccess = newMap
+
+	// order is expected to be the same from epoch to epoch


VM is stable in every aspect of it given that inputs/outputs are the same.

roman-khimov · 2024-12-15T13:43:40Z

pkg/innerring/processors/netmap/process_epoch.go

+		return fmt.Errorf("can't get containers list: %w", err)
+	}
+
+	for _, cID := range cids {


Works for now.

carpawell · 2024-12-18T17:42:35Z

Added meta validity based on blocks, not epochs.

pkg/innerring/processors/container/process_container.go

roman-khimov · 2024-12-18T20:13:20Z

pkg/services/object/put/distributed.go

+				continue
+			}
+
+			if !sig.Verify(t.objSharedMeta) {


This works for node in N+1 and peers in N, but not the other way around (if not in N objSharedMeta is for N, but the first signature from peers will already be for N+1).

@roman-khimov, hm, that is right. but i am not sure what else can be done here, we cannot collect two different signatures since we cannot send half signatures for the "outdated" nodes and half for the "actual" one, server-side always needs to get a single version for vub (or we need contract updates again). i can suggest sending three signatures and the server can always try currentEpoch + 1 (btw, this is even better, since we can get an object in the last block of the epoch, and then handling may break in the next block on the contract side), which handles every kind of epoch divergence that is not bigger than 1 epoch

Signed-off-by: Pavel Karpy <[email protected]>

Follows nspcc-dev/neofs-contract#438. Signed-off-by: Pavel Karpy <[email protected]>

Signed-off-by: Pavel Karpy <[email protected]>

After nspcc-dev/neofs-contract#438 Container contract now has API for container-side placement/verification operations. But building placement vectors and updating when needed is still a complex operation that should be done on a Go application side. This commit adds such a responsibility for Alphabet nodes. For simplicity, updating is done every epoch and once for _every_ container if netmap has been changed. Additional optimization can be considered. Signed-off-by: Pavel Karpy <[email protected]>

Follows nspcc-dev/neofs-contract#414. Signed-off-by: Pavel Karpy <[email protected]>

Refs nspcc-dev/neofs-contract#414. Push meta only if consistency policy asks for it Signed-off-by: Pavel Karpy <[email protected]>

According to nspcc-dev/neofs-api#309, "strict" must wait for meta-data handling on the contract-side and every PUT request is responded only with transaction acceptance. Signed-off-by: Pavel Karpy <[email protected]>

Will be helpful for new meta-data handling policies. Signed-off-by: Pavel Karpy <[email protected]>

It is the most secure way to ensure changes are done in sync and either all at once, or none of them. It is still can be a problem to update too long placement vectors in a single TX since there is always a certain limit, but it can be reconsidered separately. Signed-off-by: Pavel Karpy <[email protected]>

Signed-off-by: Pavel Karpy <[email protected]>

It now may have additional arg that enables meta-on-chain support and be named, see nspcc-dev/neofs-contract#451. Closes #2877. Signed-off-by: Pavel Karpy <[email protected]>

Add logs about successful operation. Also, log empty signatures and do not try to continue sending when there are no signatures. Signed-off-by: Pavel Karpy <[email protected]>

It is more natural for on-chain operations. Also, it requires handling sync problems when different blocks are used for the same objects on different nodes: two signatures are always attached with epoch duration difference. Refs nspcc-dev/neofs-contract#451. Signed-off-by: Pavel Karpy <[email protected]>

roman-khimov · 2024-12-23T19:35:51Z

pkg/morph/client/container/nodes.go

+
+	// arrays take parts in transaction that should be multi-singed, so order
+	// is important to be the same
+	slices.SortFunc(res, func(a, b []byte) int {


slices.SortFunc(res, bytes.Compare)

carpawell self-assigned this Dec 10, 2024

carpawell force-pushed the feat/push-object-meta-to-contracts branch from 83dbf8a to da38df6 Compare December 10, 2024 21:12

carpawell force-pushed the feat/push-object-meta-to-contracts branch 5 times, most recently from 1c01505 to aa415c2 Compare December 11, 2024 17:01

carpawell commented Dec 11, 2024

View reviewed changes

carpawell marked this pull request as ready for review December 11, 2024 17:07

carpawell requested review from roman-khimov and cthulhu-rider as code owners December 11, 2024 17:07

cthulhu-rider reviewed Dec 12, 2024

View reviewed changes

carpawell force-pushed the feat/push-object-meta-to-contracts branch from aa415c2 to 8ce846c Compare December 12, 2024 15:31

roman-khimov reviewed Dec 15, 2024

View reviewed changes

carpawell force-pushed the feat/push-object-meta-to-contracts branch 2 times, most recently from 59fb40b to 74d32f1 Compare December 18, 2024 17:41

carpawell requested review from roman-khimov and cthulhu-rider December 18, 2024 17:41

roman-khimov reviewed Dec 18, 2024

View reviewed changes

carpawell added 7 commits December 23, 2024 12:55

go.mod: update contracts to the latest version

1775475

Signed-off-by: Pavel Karpy <[email protected]>

morph: add method for container's members updates

59cebb3

Follows nspcc-dev/neofs-contract#438. Signed-off-by: Pavel Karpy <[email protected]>

morph/notary: Allow waiting for notary request inclusion

4e14e43

Signed-off-by: Pavel Karpy <[email protected]>

morph/container support new meta-data method

befd00d

Follows nspcc-dev/neofs-contract#414. Signed-off-by: Pavel Karpy <[email protected]>

node/object/put: push object's meta to contract

1af79c4

Refs nspcc-dev/neofs-contract#414. Push meta only if consistency policy asks for it Signed-off-by: Pavel Karpy <[email protected]>

morph,node/object/put: support "strict" meta-data policy

212a031

According to nspcc-dev/neofs-api#309, "strict" must wait for meta-data handling on the contract-side and every PUT request is responded only with transaction acceptance. Signed-off-by: Pavel Karpy <[email protected]>

carpawell added 5 commits December 23, 2024 13:26

morph/static: make it possible to wait for TX inclusion

95bbed8

Will be helpful for new meta-data handling policies. Signed-off-by: Pavel Karpy <[email protected]>

ir/container: drop useless parameter vars

ba2f720

Signed-off-by: Pavel Karpy <[email protected]>

morph: support overloads for container.Put contract method

c6c6aea

It now may have additional arg that enables meta-on-chain support and be named, see nspcc-dev/neofs-contract#451. Closes #2877. Signed-off-by: Pavel Karpy <[email protected]>

node/object/put: improve metadata logging

70bd9ea

Add logs about successful operation. Also, log empty signatures and do not try to continue sending when there are no signatures. Signed-off-by: Pavel Karpy <[email protected]>

carpawell force-pushed the feat/push-object-meta-to-contracts branch from 74d32f1 to 8679044 Compare December 23, 2024 12:12

carpawell requested a review from roman-khimov December 23, 2024 12:15

carpawell force-pushed the feat/push-object-meta-to-contracts branch from 8679044 to ae6e37d Compare December 23, 2024 13:15

roman-khimov approved these changes Dec 23, 2024

View reviewed changes

roman-khimov merged commit 3a10bc1 into master Dec 23, 2024
19 of 22 checks passed

roman-khimov deleted the feat/push-object-meta-to-contracts branch December 23, 2024 19:43

carpawell mentioned this pull request Dec 23, 2024

Feat/add big objects support for meta on chain #3063

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat/push object meta to contracts #3044

Feat/push object meta to contracts #3044

carpawell commented Dec 10, 2024

codecov bot commented Dec 10, 2024 •

edited

Loading

carpawell Dec 11, 2024

roman-khimov Dec 15, 2024

carpawell commented Dec 11, 2024

cthulhu-rider left a comment

cthulhu-rider Dec 12, 2024

carpawell Dec 12, 2024

cthulhu-rider Dec 12, 2024

carpawell Dec 12, 2024

cthulhu-rider Dec 12, 2024

carpawell Dec 12, 2024

cthulhu-rider Dec 12, 2024

cthulhu-rider Dec 12, 2024

cthulhu-rider Dec 12, 2024

cthulhu-rider Dec 12, 2024

carpawell Dec 12, 2024

cthulhu-rider Dec 12, 2024

cthulhu-rider Dec 12, 2024

carpawell Dec 12, 2024

cthulhu-rider Dec 12, 2024

carpawell Dec 12, 2024

carpawell commented Dec 12, 2024

roman-khimov Dec 15, 2024

roman-khimov Dec 15, 2024

carpawell commented Dec 18, 2024

roman-khimov Dec 18, 2024

carpawell Dec 23, 2024

roman-khimov Dec 23, 2024

Feat/push object meta to contracts #3044

Feat/push object meta to contracts #3044

Conversation

carpawell commented Dec 10, 2024

codecov bot commented Dec 10, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carpawell commented Dec 11, 2024

cthulhu-rider left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carpawell commented Dec 12, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carpawell commented Dec 18, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Dec 10, 2024 •

edited

Loading