-
Notifications
You must be signed in to change notification settings - Fork 471
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
network: Limit message length based on Tag #5388
Changes from 57 commits
a9131bb
6ce8cea
94dbf71
0d17dc6
73be798
70ee7e3
8153660
67d89db
a181944
5ea2771
03df5fe
0646f7f
50a267d
d6e6366
ecda744
f7b369e
4c25f21
64288a5
66cbf61
f82c316
4580863
45b0d65
e9ba543
dbf99c2
f6b4101
895d9a7
8d37cbb
3c11c66
ffb02ba
fb8c934
49e0e5d
bfff077
82efc8c
5e3a434
a3df2d2
3289d45
5d923e2
4925a33
6c8b13d
74ce8f1
1e57b2b
c074625
a1b01cc
5867661
0c6d583
f610a9b
cb3029b
1f008bb
a500239
4bd07f5
e52b870
03f4b01
e563c69
02364e1
289b062
e0cc3ee
3e84468
5de753b
9db27dd
7d31ac7
b64ca5b
ffcf43e
52cc1d8
036f012
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -81,6 +81,14 @@ const ConfigurableConsensusProtocolsFilename = "consensus.json" | |
// do not expose in normal config so it is not in code generated local_defaults.go | ||
const defaultRelayGossipFanout = 8 | ||
|
||
// MaxGenesisIDLen is the maximum length of the genesis ID set for purpose of setting | ||
// allocbounds on structs containing GenesisID and for purposes of calculating MaxSize functions | ||
// on those types. Current value is larger than the existing network IDs and the ones used in testing | ||
const MaxGenesisIDLen = 128 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why would one need to change this parameter? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It should never be necessary. The reason why this is so large is as I mentioned in the comment that there are tests that depend on the test name being passed around as the genesisID |
||
|
||
// MaxEvalDeltaTotalLogSize is the maximum size of the sum of all log sizes in a single eval delta. | ||
const MaxEvalDeltaTotalLogSize = 1024 | ||
|
||
// LoadConfigFromDisk returns a Local config structure based on merging the defaults | ||
// with settings loaded from the config file from the custom dir. If the custom file | ||
// cannot be loaded, the default config is returned (with the error from loading the | ||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,11 +20,38 @@ import ( | |
"fmt" | ||
|
||
"github.com/algorand/go-algorand/crypto" | ||
"github.com/algorand/msgp/msgp" | ||
) | ||
|
||
// Proof is used to convince a verifier about membership of leaves: h0,h1...hn | ||
// at indexes i0,i1...in on a tree. The verifier has a trusted value of the tree | ||
// root hash. | ||
// Path is bounded by MaxNumLeaves since there could be multiple reveals, and | ||
// given the distribution of the elt positions and the depth of the tree, | ||
// the path length can increase up to 2^MaxTreeDepth / 2 | ||
// | ||
// . Consider two different reveals for the same tree: | ||
// . | ||
// . z5 | ||
// . z3 z4 | ||
// . y z z1 z2 | ||
// . q r s t u v w x | ||
// . a b c d e f g h i j k l m n o p | ||
// . ^ | ||
// . hints: [a, r, z, z4] | ||
// . len(hints) = 4 | ||
// . You need a to combine with b to get q, need r to combine with the computed q and get y, and so on. | ||
// . The worst case is this: | ||
// . | ||
// . z5 | ||
// . z3 z4 | ||
// . y z z1 z2 | ||
// . q r s t u v w x | ||
// . a b c d e f g h i j k l m n o p | ||
// . ^ ^ ^ ^ ^ ^ ^ ^ | ||
// . | ||
// . hints: [b, d, e, g, j, l, m, o] | ||
// . len(hints) = 2^4/2 | ||
type Proof struct { | ||
_struct struct{} `codec:",omitempty,omitemptyarray"` | ||
|
||
|
@@ -41,12 +68,34 @@ type Proof struct { | |
// SingleLeafProof is used to convince a verifier about membership of a specific | ||
// leaf h at index i on a tree. The verifier has a trusted value of the tree | ||
// root hash. it corresponds to merkle verification path. | ||
// The msgp directive belows tells msgp to not generate SingleLeafProofMaxSize method since we have one manually defined in this file. | ||
// | ||
//msgp:maxsize ignore SingleLeafProof | ||
type SingleLeafProof struct { | ||
_struct struct{} `codec:",omitempty,omitemptyarray"` | ||
|
||
Proof | ||
} | ||
|
||
// ProofMaxSizeByElements returns the maximum msgp encoded size of merklearray.Proof structs containing n signatures | ||
// This is necessary because the allocbounds on the proof are actual theoretical bounds but for calculating maximum | ||
// proof size for individual message types we have smaller valid bounds. | ||
// Exported because it's used in the stateproof package for ensuring that SigPartProof constant is correct size. | ||
func ProofMaxSizeByElements(n int) (s int) { | ||
s = 1 + 4 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am very worried about having such fine-grained calculations done manually. Still considering this, and making sure the calculations are correct. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's actually not fully manual. It's a modification of the autogenerated test: here https://github.com/algorand/go-algorand/pull/5388/files#diff-add6a5ef46c2629eb7484bf3113df670d70cfdb6661ca69e236a778a7e1cb4c4R334 and we have a test here https://github.com/algorand/go-algorand/pull/5388/files#diff-181a1af1a38a976d2ddce9a94f778520e2ad787079c3cc99dbd1b7483386aa73R248 that ensures that that auto-generated ProofMaxSize and and ProofMaxSizeByElements output the same value when the allocbound is used as the input There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The test is very good and reliable. However, the mapping of the function In this particular case, this function is not needed. It can be implemented by setting a constant bound directly to the Proof field in:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah but we do have a pattern for doing this manual implementation already for basics.MicroAlgos and this solution will continue working if we ever use SingleLeafProof elsewhere. Finding it's |
||
// Calculating size of slice: z.Path | ||
s += msgp.ArrayHeaderSize + n*(crypto.GenericDigestMaxSize()) | ||
s += 4 + crypto.HashFactoryMaxSize() + 3 + msgp.Uint8Size | ||
return | ||
} | ||
|
||
// SingleLeafProofMaxSize returns the maximum msgp encoded size of proof verifying a single leaf | ||
// It is manually defined here instead of letting msgp do it since we cannot annotate the embedded Proof struct | ||
// with maxtotalbytes for msgp to autogenerate it. | ||
func SingleLeafProofMaxSize() int { | ||
return ProofMaxSizeByElements(MaxEncodedTreeDepth) | ||
} | ||
|
||
// GetFixedLengthHashableRepresentation serializes the proof into a sequence of bytes. | ||
// it basically concatenates the elements of the verification path one after another. | ||
// The function returns a fixed length array for each hash function. which is 1 + MaxEncodedTreeDepth * digestsize | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small nit: we should probably start using crypto/rand more in test cases like this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This example is so tiny it doesn't matter