Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

netmap: Document memory model #276

Merged
merged 3 commits into from
Jan 30, 2023

Conversation

cthulhu-rider
Copy link
Contributor

@cthulhu-rider cthulhu-rider commented Sep 30, 2022

I hope this will help us to carefully migrate contract data on update in future. Format suggestions are welcome. I thought about .md.

Copy link
Member

@AnnaShaleva AnnaShaleva left a comment

Choose a reason for hiding this comment

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

The config of netmap contract must be updated wrt the same comments, notifications specified in the config must exactly match the ones used in the contract, because we have neo-project/neo#2810.

netmap/doc.go Outdated
type: Integer
- name: publicKey
type: PublicKey
UpdateState
Copy link
Member

Choose a reason for hiding this comment

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

It's not UpdateState, it's UpdateStateSuccess:

runtime.Notify("UpdateStateSuccess", publicKey, state)

netmap/doc.go Outdated
- name: publicKey
type: PublicKey
UpdateState
- name: state
Copy link
Member

Choose a reason for hiding this comment

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

First parameter is public key:

runtime.Notify("UpdateStateSuccess", publicKey, state)
.

netmap/doc.go Outdated
AddPeer
- name: nodeInfo
type: ByteArray
AddPeer
Copy link
Member

Choose a reason for hiding this comment

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

Ditto, AddPeerSuccess:

runtime.Notify("AddPeerSuccess", interop.PublicKey(publicKey))

netmap/doc.go Outdated
type: ByteArray
AddPeer
- name: nodeInfo
type: ByteArray
Copy link
Member

@AnnaShaleva AnnaShaleva Oct 3, 2022

Choose a reason for hiding this comment

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

Type is PublicKey:

runtime.Notify("AddPeerSuccess", interop.PublicKey(publicKey))

netmap/doc.go Outdated
type: ByteArray
AddPeer
- name: nodeInfo
type: ByteArray

UpdateState notification. This notification is produced when a Storage node wants
to change its state (go offline) by invoking UpdateState method. Supported
states: (2) -- offline.
Copy link
Member

Choose a reason for hiding this comment

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

There are three states now, we need to fix the doc.

@cthulhu-rider
Copy link
Contributor Author

@AnnaShaleva thanks for your comments, they are relevant but outside the scope of this PR. I've created a bug #279.

carpawell
carpawell previously approved these changes Oct 11, 2022
netmap/doc.go Outdated
Contract records set of network parties representing the network map. Current
network map is updated on each epoch tick. Contract also holds limited number
of previous network maps (SNAPSHOT_LIMIT). Timestamped network maps are
called snapshots. Snapshots are identified by the numerical ring [0:SNAPSHOT_LIMIT).
Copy link
Member

Choose a reason for hiding this comment

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

what does "ring" mean?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Circular list.

netmap/doc.go Outdated
called snapshots. Snapshots are identified by the numerical ring [0:SNAPSHOT_LIMIT).

# Network map candidates
Contract stores information about the network parties which were requested
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Contract stores information about the network parties which were requested
Contract stores information about the network parties which were accepted

sounds more accurate IMO

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They are not accepted to the network map since they are candidates.

Start package docs with commonly used format `Package netmap ...`.

Signed-off-by: Leonard Lyubich <[email protected]>
Add comments (outside the docs) with key-value storage structure and
some explanations. The memory mode will allow to more precisely
understand and migrate the contract storage.

Signed-off-by: Leonard Lyubich <[email protected]>
netmap/doc.go Outdated Show resolved Hide resolved
netmap/doc.go Outdated Show resolved Hide resolved
Actualize storage node's structure type and clarify this is a type not
to cause misunderstanding.

Signed-off-by: Leonard Lyubich <[email protected]>
@roman-khimov roman-khimov merged commit f5fbf2e into nspcc-dev:master Jan 30, 2023
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.

4 participants