Skip to content
This repository has been archived by the owner on Jun 6, 2023. It is now read-only.

Bound length of strings and arrays in state #897

Closed
2 of 4 tasks
anorth opened this issue Aug 5, 2020 · 3 comments · Fixed by #1076
Closed
2 of 4 tasks

Bound length of strings and arrays in state #897

anorth opened this issue Aug 5, 2020 · 3 comments · Fixed by #1076
Assignees
Labels
P1 High priority, required for basic network functionality and growth robustness Related to correctness

Comments

@anorth
Copy link
Member

anorth commented Aug 5, 2020

A few items in state are strings or byte arrays that are provided from off chain. We should bound the length of these strings to a tight reasonable bound, else we could incur unwanted de/serialization cost as these things are read and written (potentially by parties other than the party controlling the value).

  • DealProposal.Label: 100s of bytes?
  • MinerInfo.PeerID: 100s of bytes?
  • MinerInfo.Multiaddrs: array length of 10s? each value 100s of bytes?
  • Multisig.Signers: array length of 100s?

The Address type is limited on CBOR decoding to 64 bytes, though it should also check on encoding.

BigIntegers are limited to 128 bytes, which is probably ok from a serde point of view (though maybe not for arithmetic, #866).

Related to #604

@anorth anorth added the P1 High priority, required for basic network functionality and growth label Aug 5, 2020
@anorth anorth mentioned this issue Aug 7, 2020
@Stebalien
Copy link
Member

#920 currently deals with MinerInfo.PeerID and MinerInfo.Multiaddrs.

@anorth anorth added the robustness Related to correctness label Aug 19, 2020
@anorth
Copy link
Member Author

anorth commented Aug 31, 2020

@Stebalien done?

@Stebalien
Copy link
Member

Not quite. Still need deal label, and signers.

I'm picking 256 for the deal label, same for max signers.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
P1 High priority, required for basic network functionality and growth robustness Related to correctness
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants