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

Implement some of good practices for the contracts #307

Closed
AnnaShaleva opened this issue Jan 27, 2023 · 1 comment · Fixed by #405
Closed

Implement some of good practices for the contracts #307

AnnaShaleva opened this issue Jan 27, 2023 · 1 comment · Fixed by #405
Labels
enhancement Improving existing functionality I4 No visible changes S2 Regular significance U4 Nothing urgent
Milestone

Comments

@AnnaShaleva
Copy link
Member

Some of our contracts contain issues that need to be refactored:

  1. Replace common.BytesEqual usages with built-in (a Hash160) Equals(b interface{}) bool where possible (see
    if !common.BytesEqual(caller, []byte(gas.Hash)) && !common.BytesEqual(caller, []byte(neo.Hash)) {
    as an example). The same thing can be done for Hash256 comparison cases.
  2. Use pre-defined constants where appropriate (interop.Hash160Len, interop.PublicKeyCompressedLen, etc.). There are still several cases where numbers are use instead (see
    containerID := eACL[offset : offset+32]
    as an example).
  3. Use the most specific parameter types for method signatures where appropriate (e.g. interop.Hash160 instead of []byte), see
    func Bind(user []byte, keys []interop.PublicKey) {
    as an example.

Ref. #304.

AnnaShaleva added a commit that referenced this issue Jan 31, 2023
AnnaShaleva added a commit that referenced this issue Jan 31, 2023
Part 1 from #307.

Signed-off-by: Anna Shaleva <anna@nspcc.ru>
AnnaShaleva added a commit that referenced this issue Jan 31, 2023
Part 2 from #307.

Signed-off-by: Anna Shaleva <anna@nspcc.ru>
@AnnaShaleva
Copy link
Member Author

Suggestions 1 and 2 are implemented in #317. Suggestion 3 is left to be implemented in a separate PR, as it requires careful review of the contract API's users in the node's code.

@roman-khimov roman-khimov added this to the v0.18.0 milestone Jan 31, 2023
AnnaShaleva added a commit that referenced this issue Feb 1, 2023
Part 2 from #307.

Signed-off-by: Anna Shaleva <anna@nspcc.ru>
@roman-khimov roman-khimov modified the milestones: v0.18.0, v0.19.0 Sep 26, 2023
@roman-khimov roman-khimov modified the milestones: v0.19.0, v0.20.0 Nov 22, 2023
@roman-khimov roman-khimov added enhancement Improving existing functionality I4 No visible changes U4 Nothing urgent S2 Regular significance and removed refactor labels Dec 20, 2023
roman-khimov added a commit that referenced this issue Jun 18, 2024
Part of #307, makes code a bit more efficient.

Signed-off-by: Roman Khimov <roman@nspcc.ru>
roman-khimov added a commit that referenced this issue Jun 18, 2024
Part of #307. NeoFS node currently passes and parses byte arrays here which is
a compatible behavior.

Signed-off-by: Roman Khimov <roman@nspcc.ru>
roman-khimov added a commit that referenced this issue Jun 18, 2024
Part of #307, makes code a bit more efficient.

Signed-off-by: Roman Khimov <roman@nspcc.ru>
roman-khimov added a commit that referenced this issue Jun 18, 2024
Part of #307. NeoFS node currently passes and parses byte arrays here which is
a compatible behavior.

Signed-off-by: Roman Khimov <roman@nspcc.ru>
roman-khimov added a commit that referenced this issue Jun 18, 2024
roman-khimov added a commit that referenced this issue Jun 20, 2024
Prefix-less storage is dangerous, can be easily mixed with something else
for different calls.

Signed-off-by: Roman Khimov <roman@nspcc.ru>
roman-khimov added a commit that referenced this issue Jun 21, 2024
Prefix-less storage is dangerous, can be easily mixed with something else
for different calls.

Signed-off-by: Roman Khimov <roman@nspcc.ru>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improving existing functionality I4 No visible changes S2 Regular significance U4 Nothing urgent
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants