From 5a0f9198bb0f0121321f43eac9a094c4cfe23991 Mon Sep 17 00:00:00 2001 From: vyzaldysanchez Date: Wed, 18 Dec 2024 11:48:27 -0400 Subject: [PATCH 1/5] Fixes `padWorkflowName()` --- pkg/capabilities/consensus/ocr3/types/aggregator.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pkg/capabilities/consensus/ocr3/types/aggregator.go b/pkg/capabilities/consensus/ocr3/types/aggregator.go index af86026ed..8f1b01aa0 100644 --- a/pkg/capabilities/consensus/ocr3/types/aggregator.go +++ b/pkg/capabilities/consensus/ocr3/types/aggregator.go @@ -1,6 +1,7 @@ package types import ( + "encoding/hex" "strings" ocrcommon "github.com/smartcontractkit/libocr/commontypes" @@ -27,7 +28,10 @@ type Metadata struct { // the json schema allows for a variable length string <= len(10) // pad with trailing spaces to meet the contract requirements func (m *Metadata) padWorkflowName() { - if len(m.WorkflowName) < 10 { + b, err := hex.DecodeString(m.WorkflowName) + if err == nil && len(b) < 10 { + m.WorkflowName = hex.EncodeToString(append(b, make([]byte, 10-len(b))...)) + } else if len(m.WorkflowName) < 10 { suffix := strings.Repeat(" ", 10-len(m.WorkflowName)) m.WorkflowName += suffix } From 2665cb6ddc007cf3f18c692264b8709cb25c3e7b Mon Sep 17 00:00:00 2001 From: vyzaldysanchez Date: Thu, 19 Dec 2024 00:49:13 -0400 Subject: [PATCH 2/5] Fixes `padWorkflowName()` --- .../consensus/ocr3/types/aggregator.go | 9 +++++--- .../consensus/ocr3/types/aggregator_test.go | 23 ++++++++++++------- 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/pkg/capabilities/consensus/ocr3/types/aggregator.go b/pkg/capabilities/consensus/ocr3/types/aggregator.go index 8f1b01aa0..bfe507a01 100644 --- a/pkg/capabilities/consensus/ocr3/types/aggregator.go +++ b/pkg/capabilities/consensus/ocr3/types/aggregator.go @@ -25,13 +25,16 @@ type Metadata struct { } // the contract requires exactly 10 bytes for the workflow name -// the json schema allows for a variable length string <= len(10) -// pad with trailing spaces to meet the contract requirements +// the resulting workflow name should be up to 10 bytes long +// so pad accordingly to meet the contract requirements func (m *Metadata) padWorkflowName() { b, err := hex.DecodeString(m.WorkflowName) if err == nil && len(b) < 10 { - m.WorkflowName = hex.EncodeToString(append(b, make([]byte, 10-len(b))...)) + // Each byte is 2 characters, so we need to pad with 0s + neededBytes := append(b, make([]byte, 10-len(b))...) + m.WorkflowName = hex.EncodeToString(neededBytes) } else if len(m.WorkflowName) < 10 { + // Pad with spaces suffix := strings.Repeat(" ", 10-len(m.WorkflowName)) m.WorkflowName += suffix } diff --git a/pkg/capabilities/consensus/ocr3/types/aggregator_test.go b/pkg/capabilities/consensus/ocr3/types/aggregator_test.go index dce0179b3..e1cd6855a 100644 --- a/pkg/capabilities/consensus/ocr3/types/aggregator_test.go +++ b/pkg/capabilities/consensus/ocr3/types/aggregator_test.go @@ -16,25 +16,32 @@ func TestMetadata_padWorkflowName(t *testing.T) { want string }{ { - name: "padWorkflowName 1", + name: "padWorkflowName hex with 9 bytes", fields: fields{ - WorkflowName: "123456789", + WorkflowName: "ABCD1234EF567890AB", }, - want: "123456789 ", + want: "abcd1234ef567890ab00", }, { - name: "padWorkflowName 0", + name: "padWorkflowName hex with 5 bytes", fields: fields{ - WorkflowName: "1234567890", + WorkflowName: "1234ABCD56", }, - want: "1234567890", + want: "1234abcd560000000000", }, { - name: "padWorkflowName 10", + name: "padWorkflowName empty", fields: fields{ WorkflowName: "", }, - want: " ", + want: "00000000000000000000", + }, + { + name: "padWorkflowName non-hex string", + fields: fields{ + WorkflowName: "not-hex", + }, + want: "not-hex ", }, } for _, tt := range tests { From e48f438fdca5df54826d6444dd086621714c9c6a Mon Sep 17 00:00:00 2001 From: vyzaldysanchez Date: Thu, 19 Dec 2024 12:20:02 -0400 Subject: [PATCH 3/5] Fixes `padWorkflowName()` --- pkg/capabilities/consensus/ocr3/types/aggregator.go | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/pkg/capabilities/consensus/ocr3/types/aggregator.go b/pkg/capabilities/consensus/ocr3/types/aggregator.go index bfe507a01..642d5d164 100644 --- a/pkg/capabilities/consensus/ocr3/types/aggregator.go +++ b/pkg/capabilities/consensus/ocr3/types/aggregator.go @@ -1,7 +1,6 @@ package types import ( - "encoding/hex" "strings" ocrcommon "github.com/smartcontractkit/libocr/commontypes" @@ -28,14 +27,9 @@ type Metadata struct { // the resulting workflow name should be up to 10 bytes long // so pad accordingly to meet the contract requirements func (m *Metadata) padWorkflowName() { - b, err := hex.DecodeString(m.WorkflowName) - if err == nil && len(b) < 10 { - // Each byte is 2 characters, so we need to pad with 0s - neededBytes := append(b, make([]byte, 10-len(b))...) - m.WorkflowName = hex.EncodeToString(neededBytes) - } else if len(m.WorkflowName) < 10 { - // Pad with spaces - suffix := strings.Repeat(" ", 10-len(m.WorkflowName)) + // it should have 10 hex bytes, so 20 characters total + if len(m.WorkflowName) < 20 { + suffix := strings.Repeat("0", 20-len(m.WorkflowName)) m.WorkflowName += suffix } } From 8b34b1e6df970d150a99d298ad6b22ec863e3ea9 Mon Sep 17 00:00:00 2001 From: vyzaldysanchez Date: Thu, 19 Dec 2024 12:22:39 -0400 Subject: [PATCH 4/5] Updates comments on `Metadata` struct --- pkg/capabilities/consensus/ocr3/types/aggregator.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/capabilities/consensus/ocr3/types/aggregator.go b/pkg/capabilities/consensus/ocr3/types/aggregator.go index 642d5d164..6484a5fbc 100644 --- a/pkg/capabilities/consensus/ocr3/types/aggregator.go +++ b/pkg/capabilities/consensus/ocr3/types/aggregator.go @@ -13,14 +13,14 @@ const MetadataFieldName = "INTERNAL_METADATA" type Metadata struct { Version uint32 // 1 byte - ExecutionID string // 32 hex bytes + ExecutionID string // 32 hex bytes (string len = 64) Timestamp uint32 // 4 bytes DONID uint32 // 4 bytes DONConfigVersion uint32 // 4 bytes - WorkflowID string // 32 hex bytes - WorkflowName string // 10 hex bytes - WorkflowOwner string // 20 hex bytes - ReportID string // 2 hex bytes + WorkflowID string // 32 hex bytes (string len = 64) + WorkflowName string // 10 hex bytes (string len = 20) + WorkflowOwner string // 20 hex bytes (string len = 40) + ReportID string // 2 hex bytes (string len = 4) } // the contract requires exactly 10 bytes for the workflow name From 04ab4060f103eab39af3a825c4691e7ee1f90c70 Mon Sep 17 00:00:00 2001 From: vyzaldysanchez Date: Thu, 19 Dec 2024 12:26:09 -0400 Subject: [PATCH 5/5] Fixes tests --- pkg/capabilities/consensus/ocr3/types/aggregator_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/capabilities/consensus/ocr3/types/aggregator_test.go b/pkg/capabilities/consensus/ocr3/types/aggregator_test.go index e1cd6855a..94660d103 100644 --- a/pkg/capabilities/consensus/ocr3/types/aggregator_test.go +++ b/pkg/capabilities/consensus/ocr3/types/aggregator_test.go @@ -20,14 +20,14 @@ func TestMetadata_padWorkflowName(t *testing.T) { fields: fields{ WorkflowName: "ABCD1234EF567890AB", }, - want: "abcd1234ef567890ab00", + want: "ABCD1234EF567890AB00", }, { name: "padWorkflowName hex with 5 bytes", fields: fields{ WorkflowName: "1234ABCD56", }, - want: "1234abcd560000000000", + want: "1234ABCD560000000000", }, { name: "padWorkflowName empty", @@ -41,7 +41,7 @@ func TestMetadata_padWorkflowName(t *testing.T) { fields: fields{ WorkflowName: "not-hex", }, - want: "not-hex ", + want: "not-hex0000000000000", }, } for _, tt := range tests {