Do not enforce PoV size hard limit in config #5887
Conversation
|
Should we account for some encoding overhead? |
In theory, yes, but in practice, it doesn't make sense. This limit was supposed to be a compressed PoV limit, that is, exactly what is transferred through the network. But indeed, as we don't know how much it will compress while building a block, we use it as an uncompressed PoV limit, so after the compression, it always comes out smaller with a good reserve. |
| @@ -19,6 +19,8 @@ serde = { features = ["alloc", "derive"], workspace = true } | |||
| derive_more = { workspace = true, default-features = true } | |||
| bitflags = { workspace = true } | |||
|
|
|||
| sc-network = { workspace = true } | |||
There was a problem hiding this comment.
This looks like a really heavy dependency to bring into the runtime just for this constant, not sure if the compiler is smart enough to saves us all the troubles we might have.
There was a problem hiding this comment.
Any better ideas? Since #5753 we have a dedicated constant for PoV response size, but it is also situated in a location we wouldn't want to import. Maybe we should move that constant somewhere but I'm struggling to find a good place for it. polkadot-parachain-primitives is perfect, as it relates to parachains and is shared between the node and the runtime, but it seems to me it's strange to have a network-related constant there.
There was a problem hiding this comment.
Also, this is low level networking, doesn't make sense for the runtime to depend on it.
There was a problem hiding this comment.
Any better ideas?
an alternative is in comment #5887 (comment)
This is a backport of changes to the release branch used in the current fellowship runtimes. It enables us to release new fellowship runtimes with the PoV size limit lifted, not waiting for the current major fellowship release, which has been stuck in the audit.
Quoting @bkchr (from here):
This PR aims to change the hard limit to a not-so-random constant, allowing more room for maneuvering in the future.