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

Length of frames #1

Closed
favonia opened this issue Jun 28, 2018 · 14 comments
Closed

Length of frames #1

favonia opened this issue Jun 28, 2018 · 14 comments
Labels
question Further information is requested

Comments

@favonia
Copy link
Collaborator

favonia commented Jun 28, 2018

Is there a reason not to use bounded integers for the lengths?

@favonia favonia added the question Further information is requested label Jun 28, 2018
@liyishuai
Copy link
Owner

Does Coq have a bounded integer library? I'm a bit reluctant to depend on external packages.

@lastland
Copy link
Collaborator

One way to encode bounded integers in Coq is using subset types {n: nat | n < ... }. Then by using Program Definition, etc., you can reuse most of the functions provided by nat, with obligations to prove that if the result is also a bounded integer, it is within the bound.

However, many people would suggest reconsidering the options before using that because reasoning about dependent types (for example, when doing unification, etc.) in Coq can be painful...

@liyishuai
Copy link
Owner

Also, considering that SETTINGS_MAX_FRAME_SIZE can go up to 16M, we should use binary numbers rather than nat in most cases.

@favonia
Copy link
Collaborator Author

favonia commented Jun 28, 2018

I think that Sigma type encoding {nat | ...} should work, even if not the best solution.

Re: 16M: maybe we can also define 16K and 16M to be some large enough unknown constants.

@liyishuai
Copy link
Owner

SETTINGS_MAX_FRAME_SIZE varies during runtime. Is this a reason against limiting the frame size at type level? A frame cannot be guaranteed to be valid at compile time at all.

@favonia
Copy link
Collaborator Author

favonia commented Jun 28, 2018

Well, our client or server can always fix the bound to exactly 16384, the default value.

@lastland
Copy link
Collaborator

If we treat SETTINGS_MAX_FRAME_SIZE as an opaque value, it should not matter how big it is, right? Or am I missing something?

@lastland
Copy link
Collaborator

But anyway, I do not have a strong opinion on this. If you decided to use the binary format N, remember to check out peano_ind...

@favonia
Copy link
Collaborator Author

favonia commented Jun 28, 2018

If we treat SETTINGS_MAX_FRAME_SIZE as an opaque value, it should not matter how big it is, right? Or am I missing something?

I believe that is the case. We only have to make sure proper bound-checking is enforced with respect to some "reasonable" constants.

@liyishuai
Copy link
Owner

This value can be modified by SETTINGS frames. I don't think it's proper to make it opaque.
I'm somehow ambitious to make this library general enough to reason about all implementations rather than ours.

@lastland
Copy link
Collaborator

You will want to say something like, for all values we can give to the settings, some properties hold. And reasoning about the setting should not rely on it being a particular value. Therefore, the values of settings remain abstract all the time and there is no concrete number that we need to worry about.

I am not suggesting to keep using nat though, I just don't think this should be the reason behind the decision.

@lastland
Copy link
Collaborator

And although I have heard a lot of arguments against using sigma types, I would actually be interested to see how they would play out here...

@liyishuai
Copy link
Owner

You will want to say something like, for all values we can give to the settings, some properties hold.

To be concrete, what would a running checker look like?

@liyishuai
Copy link
Owner

I think 16M is a reasonable constant limit here. The static bound-checking tells if the frame can be valid in any context, and the runtime checking decides whether this frame is valid under current context.

liyishuai added a commit that referenced this issue Jul 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants