-
Notifications
You must be signed in to change notification settings - Fork 68
dlog token driver: improve the use of mathlib #1287 #1288
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 5 commits
db61f50
db3ff41
e76b90c
bd09223
22dd218
3fbd568
12d2af0
5a3b3e9
4c0358a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| /* | ||
| Copyright IBM Corp. All Rights Reserved. | ||
| SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| package crypto | ||
|
|
||
| import "encoding/binary" | ||
|
|
||
| // AppendFixed32 appends slices prefixed with a 4-byte Little Endian length. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Little Endian this means that we support only x86 not power/z deployments? |
||
| // Format: [Len(4 bytes)][Data]... | ||
| func AppendFixed32(dst []byte, s [][]byte) []byte { | ||
| // 1. Precise Size Calculation | ||
| // We calculate the exact total growth needed (4 bytes header + data length per slice). | ||
| // This allows us to perform exactly one allocation. | ||
| const headerSize = 4 | ||
| n := 0 | ||
| for _, v := range s { | ||
| n += headerSize + len(v) | ||
| } | ||
|
|
||
| // 2. Single Growth / Allocation | ||
| // If the capacity is insufficient, we grow the slice exactly once. | ||
| // This avoids the 2x growth strategy of standard append(), saving | ||
| // potentially 25-50% memory overhead on large buffers. | ||
| if cap(dst)-len(dst) < n { | ||
| newDst := make([]byte, len(dst), len(dst)+n) | ||
| copy(newDst, dst) | ||
| dst = newDst | ||
| } | ||
|
|
||
| // 3. Append Loop (Branch-free) | ||
| for _, v := range s { | ||
| // AppendUint32 is inlinable and highly optimized in Go 1.19+ [web:22] | ||
| dst = binary.LittleEndian.AppendUint32(dst, uint32(len(v))) | ||
| dst = append(dst, v...) | ||
| } | ||
|
|
||
| return dst | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,120 @@ | ||
| /* | ||
| Copyright IBM Corp. All Rights Reserved. | ||
| SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| package crypto | ||
|
|
||
| import ( | ||
| "bytes" | ||
| "encoding/binary" | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/assert" | ||
| ) | ||
|
|
||
| // Unit Test: Verifies correct Little Endian encoding and data integrity | ||
| func TestAppendFixed32(t *testing.T) { | ||
| tests := []struct { | ||
| name string | ||
| input [][]byte | ||
| expected []byte | ||
| }{ | ||
| { | ||
| name: "Basic Join", | ||
| input: [][]byte{ | ||
| []byte("Go"), | ||
| []byte("Lang"), | ||
| }, | ||
| // Expect: [Len:2][G][o] [Len:4][L][a][n][g] | ||
| // Little Endian 2: 0x02, 0x00, 0x00, 0x00 | ||
| expected: []byte{ | ||
| 0x02, 0x00, 0x00, 0x00, 'G', 'o', | ||
| 0x04, 0x00, 0x00, 0x00, 'L', 'a', 'n', 'g', | ||
| }, | ||
| }, | ||
| { | ||
| name: "Empty Input", | ||
| input: [][]byte{}, | ||
| expected: nil, // Or empty slice depending on init | ||
| }, | ||
| { | ||
| name: "Contains Empty Slice", | ||
| input: [][]byte{ | ||
| []byte("Hi"), | ||
| {}, | ||
| }, | ||
| // Expect: [Len:2][H][i] [Len:0] | ||
| expected: []byte{ | ||
| 0x02, 0x00, 0x00, 0x00, 'H', 'i', | ||
| 0x00, 0x00, 0x00, 0x00, | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| // Using nil as dst to force new allocation | ||
| result := AppendFixed32(nil, tt.input) | ||
| assert.Equal(t, tt.expected, result) | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| // Verification Test: Reusing an existing buffer | ||
| func TestAppendFixed32_ReuseBuffer(t *testing.T) { | ||
| buffer := make([]byte, 0, 1024) | ||
| buffer = append(buffer, 0xFF) // Simulating existing data (dirty buffer) | ||
|
|
||
| input := [][]byte{[]byte("A")} | ||
| result := AppendFixed32(buffer, input) | ||
|
|
||
| // Check that we didn't lose the prefix 0xFF | ||
| assert.Equal(t, byte(0xFF), result[0]) | ||
| // Check the new data starts at index 1 | ||
| // Len: 1 (0x01 00 00 00) + 'A' | ||
| expectedPayload := []byte{0x01, 0x00, 0x00, 0x00, 'A'} | ||
| assert.Equal(t, expectedPayload, result[1:]) | ||
| } | ||
|
|
||
| // --- Benchmarks --- | ||
|
|
||
| // Setup helper for benchmarks | ||
| func generateBenchmarkData(count, size int) [][]byte { | ||
| data := make([][]byte, count) | ||
| for i := 0; i < count; i++ { | ||
| data[i] = bytes.Repeat([]byte{'a'}, size) | ||
| } | ||
| return data | ||
| } | ||
|
|
||
| // Optimized Approach | ||
| func BenchmarkAppendFixed32(b *testing.B) { | ||
| // Scenario: 1000 items, 256 bytes each (~250KB total) | ||
| data := generateBenchmarkData(1000, 256) | ||
|
|
||
| b.ReportAllocs() | ||
| b.ResetTimer() | ||
|
|
||
| for i := 0; i < b.N; i++ { | ||
| // Use nil to strictly measure allocation of the result | ||
| _ = AppendFixed32(nil, data) | ||
| } | ||
| } | ||
|
|
||
| // Comparison: Naive Loop (No pre-calculation) | ||
| func BenchmarkAppendFixed32_Naive(b *testing.B) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you please add doc to this one? |
||
| data := generateBenchmarkData(1000, 256) | ||
|
|
||
| b.ReportAllocs() | ||
| b.ResetTimer() | ||
|
|
||
| for i := 0; i < b.N; i++ { | ||
| var dst []byte | ||
| for _, v := range data { | ||
| dst = binary.LittleEndian.AppendUint32(dst, uint32(len(v))) | ||
| dst = append(dst, v...) | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,11 +7,11 @@ SPDX-License-Identifier: Apache-2.0 | |
| package common | ||
|
|
||
| import ( | ||
| "bytes" | ||
| "encoding/hex" | ||
| "hash" | ||
|
|
||
| math "github.com/IBM/mathlib" | ||
| "github.com/hyperledger-labs/fabric-smart-client/pkg/utils/errors" | ||
| "github.com/hyperledger-labs/fabric-token-sdk/token/core/common/crypto" | ||
| ) | ||
|
|
||
| // Separator is used to delimit to end an array of bytes. | ||
|
|
@@ -28,11 +28,21 @@ func (a *G1Array) Bytes() ([]byte, error) { | |
| if e == nil { | ||
| return nil, errors.Errorf("failed to marshal array of G1") | ||
| } | ||
| st := hex.EncodeToString(e.Bytes()) | ||
| raw[i] = []byte(st) | ||
| raw[i] = e.Bytes() | ||
| } | ||
| // join the serialization of the group elements with the predefined separator. | ||
| return bytes.Join(raw, []byte(Separator)), nil | ||
| return crypto.AppendFixed32([]byte{}, raw), nil | ||
| } | ||
|
|
||
| func (a *G1Array) BytesTo(b []byte) ([]byte, error) { | ||
| raw := make([][]byte, len([]*math.G1(*a))) | ||
| for i, e := range []*math.G1(*a) { | ||
| if e == nil { | ||
| return nil, errors.Errorf("failed to marshal array of G1") | ||
| } | ||
| raw[i] = e.Bytes() | ||
| } | ||
| clear(b) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What does the clear do? Is that ok to clear the "b" here? what happen if the caller needs it? Maybe to add flag to clear and this will be clear to the caller.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What if "b" is nil? |
||
| return crypto.AppendFixed32(b, raw), nil | ||
| } | ||
|
|
||
| // GetG1Array takes a series of G1 elements and returns the corresponding array | ||
|
|
@@ -44,3 +54,12 @@ func GetG1Array(elements ...[]*math.G1) *G1Array { | |
| a := G1Array(array) | ||
| return &a | ||
| } | ||
|
|
||
| func HashG1Array(h hash.Hash, elements ...*math.G1) []byte { | ||
| h.Reset() | ||
|
|
||
| for _, e := range elements { | ||
| h.Write(e.Bytes()) | ||
| } | ||
| return h.Sum(nil) | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is that something that we need to tune during testing? or does this fix? (does it worth to have it in config file)?