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

Contract configuration constraints #314

Closed
AnnaShaleva opened this issue Jan 27, 2023 · 4 comments
Closed

Contract configuration constraints #314

AnnaShaleva opened this issue Jan 27, 2023 · 4 comments
Labels
config Configuration format update or breaking change enhancement Improving existing functionality I4 No visible changes S2 Regular significance U2 Seriously planned
Milestone

Comments

@AnnaShaleva
Copy link
Member

The common pattern of storing the contract configuration is to use the set of key-value records. Given the neofs contract as an example, function

func ListConfig() []record {
restricts the maximum amount of config fields to ~682 (3 stackitems for a single record, 2047 records at max). Although currently we don't have a lot of configuration values, the problem is that this list is append-only (there's no code that allows to remove values from the contract config). If someday we're going to excessively extend our configs then this problem may be solved with returning an iterator from this method.

Ref. #304.

@roman-khimov
Copy link
Member

Generic SetConfig/Config/ListConfig things are just a bad interface. Can easily be misused, impossible to track compatibility/migrations at the contract level. Of course you don't have a lot of options at _deploy time, but other than that each value should have a separate get/set function pair and contracts should deal with the storage schemas/migrations/compatibility internally.

@roman-khimov roman-khimov added enhancement Improving existing functionality refactor config Configuration format update or breaking change labels Jan 27, 2023
@roman-khimov roman-khimov added this to the v0.18.0 milestone Feb 1, 2023
@roman-khimov
Copy link
Member

Another side-effect of the current approach, BTW:

$ ./bin/neo-go contract testinvokefunction -r https://rpc3.morph.t5.fs.neo.org:51331 c4576ea5c3081dd765a17aaaa73d9352e74bdc28 config BasicIncomeRate
{
  "state": "HALT",
  "gasconsumed": "136044",
  "script": "DA9CYXNpY0luY29tZVJhdGURwB8MBmNvbmZpZwwUKNxL51KTPaeqeqFl1x0Iw6VuV8RBYn1bUg==",
  "stack": [
    {
      "type": "ByteString",
      "value": "AOH1BQ=="
    }
  ],
  "exception": null,
  "notifications": []
}

It's a number in fact.

@cthulhu-rider
Copy link
Contributor

Not a solution, but a practical relief: defining global defaults. In many cases, this will reduce the amount of data stored and the burden on the administrator.

@roman-khimov roman-khimov modified the milestones: v0.18.0, v0.19.0 Sep 21, 2023
@roman-khimov roman-khimov modified the milestones: v0.19.0, v0.20.0 Nov 22, 2023
@roman-khimov roman-khimov added I4 No visible changes U2 Seriously planned S2 Regular significance and removed refactor labels Dec 20, 2023
@roman-khimov
Copy link
Member

#372 will solve it. Then ListConfig could be removed in some future versions.

@roman-khimov roman-khimov closed this as not planned Won't fix, can't repro, duplicate, stale Dec 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
config Configuration format update or breaking change enhancement Improving existing functionality I4 No visible changes S2 Regular significance U2 Seriously planned
Projects
None yet
Development

No branches or pull requests

3 participants