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

Proposal description max length should match the max length set in the SDK #1871

Closed
jbibla opened this issue Jan 23, 2019 · 10 comments
Closed

Comments

@jbibla
Copy link
Collaborator

jbibla commented Jan 23, 2019

it would be nice if this matched the size from the SDK. but we can adjust later.

Originally posted by @jbibla in #1807

@faboweb
Copy link
Collaborator

faboweb commented Jan 24, 2019

Chris mentioned that there is only an implicit max length.

@jbibla
Copy link
Collaborator Author

jbibla commented Jan 25, 2019

implicit max length.

what does that mean?

@faboweb
Copy link
Collaborator

faboweb commented Jan 28, 2019

#1807 (comment)

There is only a max length per transaction in total.

@jbibla
Copy link
Collaborator Author

jbibla commented Jan 28, 2019

what is it? let's ensure that the description field is as large as can be to fit within this limit.

@faboweb
Copy link
Collaborator

faboweb commented Jan 28, 2019

@cwgoes what is the max size of tx?

The max length of a description is the max size of the tx minus the length of the title minus the length of the fees string etc. etc.
Hard to calc. We can add a max length that is defensive.

@cwgoes
Copy link

cwgoes commented Jan 28, 2019

I am not sure the SDK will directly enforce a maximum size (Tendermint does, but it is quite high) - rather, the maximum size will be set implicitly by a gas limit and gas charged proportional to size.

We can very easily add a separate maximum length just for validator descriptions, and that probably makes sense from the UX perspective too: cosmos/cosmos-sdk#3420 - if you would like a particular maximum length, please comment.

@faboweb
Copy link
Collaborator

faboweb commented Jan 29, 2019

Thx @cwgoes for creating the issue.

@faboweb
Copy link
Collaborator

faboweb commented Jan 29, 2019

Maybe subtract a heuristic of the size of the rest of the transaction from the max transaction size. @ebuchman what is the max transaction size?

@faboweb faboweb removed the discuss label Jan 29, 2019
@sabau
Copy link
Contributor

sabau commented Jan 29, 2019

Just as reminder @jbibla made a nice experiment with 2 messages of length:

  • 10000 throwed error
  • 5000 worked

@fedekunze
Copy link
Contributor

new length limits are:

MaxDescriptionLength = 5000
MaxTitleLength = 140

@faboweb faboweb closed this as completed Apr 5, 2019
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

No branches or pull requests

5 participants