fix: metadata padding #37
Conversation
There was a problem hiding this comment.
I would extract this logic into a separate createMetadataPadding function, but anyway after correcting the comments below, LGTM.
Though, as the comment says above the inserted padding logic, the encryptDecrypt function should correct the byte length difference. are you sure this change is needed?
Also I get errors locally because of the already fixed content-type for .txt files
|
Please review it again. I made the changes you asked for, also I updated to use Bee 1.7.0 with the new bee-factory and bee-js 5.0.0. |
|
Also fixes #38 |
There was a problem hiding this comment.
as the test shows, it is an improvement in order to be compatible with the Bee client, but I commented below, I don't think this is the proper way to pad metadata. It adds new line characters in order to not break the stringified JSON structure on load, but it does not carry any information. The deserializer should handle this in an efficient way as this library did so far. I would create a ticket about it in Bee but to be honest I would prefer more to upgrade to a newer version of mantaray already.
| } | ||
|
|
||
| const uploadData = async (data: Uint8Array): Promise<string> => { | ||
| const result = await bee.uploadData(process.env.BEE_POSTAGE, data) |
There was a problem hiding this comment.
wrong type for the first parameter, BEE_POSTAGE can be string or undefined instead of string | BatchId.
There was a problem hiding this comment.
create a function for getting process.env.BEE_POSTAGE that throws error if it is undefined.
|
|
||
| const metadataBytesSize = toBigEndianFromUint16(metadataBytes.length) | ||
| const metadataBytesSize = toBigEndianFromUint16(metadataBytes.length + padding.length) |
There was a problem hiding this comment.
I don't see any sense to add padding length to the metadataByteSize.
There was a problem hiding this comment.
Here the goal was to replicate the implementation in Bee and this is the exact same thing.
Fixes #35. Adds a test to verify that the content hash is exactly the same as what Bee generates.