[backport] consensus decode from finite decoder. #1360
Conversation
|
Perhaps, we can get a review here and make 0.28.2 with the bug fix. Since @apoelstra is offline for the next two weeks, I think @TheBlueMatt has permission to push to crates io. |
|
duh, this is more involved because we changed the MSRV in between. |
sanket1729
changed the title
As things are right now, memory exhaustion protection in `Decodable` is based on checking input-decoded lengths against arbitrary limits, and ad-hoc wrapping collection deserialization in `Take`. The problem with that are two-fold: * Potential consensus bugs due to incorrect limits. * Performance degradation when decoding nested structured, due to recursive `Take<Take<..>>` readers. This change introduces a systematic approach to the problem. A concept of a "size-limited-reader" is introduced to rely on the input data to finish at enforced limit and fail deserialization. Memory exhaustion protection is now achived by capping allocations to reasonable values, yet allowing the underlying collections to grow to accomodate rare yet legitmately oversized data (with tiny performance cost), and reliance on input data size limit. A set of simple rules allow avoiding recursive `Take` wrappers. Fix rust-bitcoin#997
sanket1729
force-pushed
the
backport-0.28
branch
from
5ffe493
to
7baa21c
Compare
|
Yea, I think that makes sense, if we can do it without breaking the MSRV. |
|
Also I assume we'll want a new branch? The branch is labeled "0.28.1"? We probably want the 0.28.x branch. |
|
I thought there were three patches in the hong-fuzz fix, they can be seen on the 0.29.2 backport. Can we have the version bump as a final patch after all the backport patches as well please? |
|
@tcharding thanks for the patches. Working on it. @TheBlueMatt , the 0.28.x is not updated to 0.28.1 tip. Will also do that and raise PR against it. |
|
CI cleared, ready for review/publication. |
There was a problem hiding this comment.
ACK a0489d4.
Patches both look good and match the 0.29 versions.
|
@tcharding can you ack? Would like a second set of eyeballs here and then we can cut a release, I think. |
|
ACK a0489d4 I verified that the first patch is the same as the original patch but with import statement changes. |
Backport for #1023. Required for #997. Addresses issues like #1359