Use cbor-gen encoding for metadata instead of go-ipld-prime#96
Use cbor-gen encoding for metadata instead of go-ipld-prime#96hannahhoward merged 1 commit intomasterfrom
Conversation
Use cbor-gen encoding for speed bump on metadata
|
@hannahhoward cbor-gen makes some strictness tradeoffs as part of its speed improvements; most notably it'll ignore trailing bytes after it's parsed the structures out that it expects. Within Filecoin, to deal with this there's an attempt to re-encode everything that comes from the outside (untrusted) to ensure that canonical forms of data structures. This is fine for use-cases, but opens security (or other) holes where canonicity matters and this isn't properly dealt with. Also cbor-gen doesn't do maps properly (according to dag-cbor and the CBOR spec). Maps are avoided in Filecoin alltogether, using tuples everywhere, and there was a proposal to remove that functionality entirely from cbor-gen so as not to provide a potentially confusing option. I've proposed at least fixing it (whyrusleeping/cbor-gen#42) but I don't see much appetite for tinkering with cbor-gen at this late stage (for Filecoin) so that's probably not going anywhere. I don't know whether either of these things impact graphsync, you're in the best position to make that call. I just wanted to make sure you're aware and are comfortable with those tradeoffs. |
* Emit events with received cids (#71) * persist received cids on channel state. * Send, Receive and Validate Restart requests (#75) * Send, Receive and Validate Requests * Initiating and Responding Tests and bug fixes (#76) * Testing for resuming data transfer work * Cleanup Push Restarts PR (#79) * cleanup of restart PR * link the peers * Tests for pull restarts (#84) * tests for pull restarts * Merge Tests cleanup work (#92) * cleanup of restart PR * cleanup timedout channels (#93) * backward compatibility of restart (#96) * backward compatibility of restart * changes and tests * more tests * better error handling for restarts * feat(message): switch to cbor map encoding (#97) switch to cbor map encoding for the 1_1 message protocol * feat(channels): setup datastore migrations (#99) setup datatransfer channels so they migrate over successfully Co-authored-by: Hannah Howard <hannah@hannahhoward.net>
Goals
Use cbor-gen encoding for speed bump working with metadata structs
Implementation
Memory profiles indicate assembling Metadata IPLD structs to encode to bytes consumed significant memory in benchmarks:
Moreover, fuzz testing has found scenarios where this can be used to panic the decoder: LeastAuthority/go-ipld-prime#2
Switching to using cbor-gen encodings provides a significant performance benefit, and cbor-gen already has significant fuzz testing.
With the change (note almost 150MB less memory used in benchmark):
