Feat/padding #1
Feat/padding #1
Conversation
…ing and unpadding
parsing/padding.go
Outdated
| ) | ||
|
|
||
| // Pad pads a general byte array in to FP32 chunks of bytes where the topmost bits of the most significant byte are 0 | ||
| func Pad(unpaddedData *[]byte) ([]types.FP32, error) { |
There was a problem hiding this comment.
You don't need to take the pointer here. The slice is a tuple of (*array, len, capacity) and is usually passed by value.
There was a problem hiding this comment.
Thanks for the clarification, that is a very nice detail to know about arrays in Go!
parsing/padding.go
Outdated
| "github.com/filecoin-project/go-data-segment/types" | ||
| ) | ||
|
|
||
| // Pad pads a general byte array in to FP32 chunks of bytes where the topmost bits of the most significant byte are 0 |
There was a problem hiding this comment.
Nit: it isn't FP32 but Fr32, it gets its name from the r of BLS-12-381: https://github.com/filecoin-project/paired/tree/master/src/bls12_381#bls12-381-instantiation.
It is a padding scheme compatible with Field over r.
| @@ -2,103 +2,103 @@ package parsing | |||
|
|
|||
| import ( | |||
| "errors" | |||
| "github.com/filecoin-project/go-data-segment/types" | |||
| "github.com/filecoin-project/go-data-segment/fr32" | |||
There was a problem hiding this comment.
I think types previously was fine, we can have more types there and now we have a repetition of the fr32.Fr32 which is a bit ugly.
There was a problem hiding this comment.
Ahh, I see why you did it, so the Bits* are nicer.
There was a problem hiding this comment.
This works as well, I would say.
parsing/padding.go
Outdated
| } | ||
|
|
||
| // Return a slice containing the next segment of unpadded data (without copying data), it will be a slice of either 32 or 33 bytes | ||
| func getChunk(bitIdx int, unpaddedData *[]byte) []byte { |
There was a problem hiding this comment.
The pointer to slice snuck through.
|
@Kubuxu I implemented the fixes suggested. I can unfortunately not set you explicitly as the reviewer as you created the PR |
No description provided.