Merged
Conversation
j-rafique
approved these changes
May 15, 2025
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR enhances SDK validation by propagating and verifying a base64 signature through the cascade flow, refactors event data handling to use typed EventData, and extends the task cache to track ActionID and transaction hash.
- Add signature generation in E2E tests and wire signature through
StartCascadeandCreateCascadeTask. - Introduce
validateAction&validateSignatureinManagerImpl, update cache storage and event handling for transaction hashes. - Refactor logging/events to use unified
LogEventand typedEventData, update adapters and types accordingly.
Reviewed Changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/system/e2e_cascade_test.go | Generate and base64‐encode signature, pass signature into StartCascade. |
| sdk/task/manager.go | Updated Manager interface and implementation to accept signature, added signature checks |
| sdk/task/helpers.go | Added validateAction & validateSignature helpers for action and signature verification |
| sdk/task/task.go | Extended BaseTask, replaced logEvent with LogEvent, adjusted cascade task flow |
| sdk/task/cache.go | Extended TaskEntry with ActionID/TxHash, updated Set signature, added UpdateTxHash |
| sdk/event/types.go | Switched event data to EventData type and updated NewEvent signature |
| sdk/event/keys.go | Added standard EventDataKey constants |
| sdk/adapters/supernodeservice/adapter.go | Updated EventLogger callback to use EventData |
| sdk/adapters/lumera/types.go | Expanded Action struct with ActionType, Metadata, Creator |
| sdk/adapters/lumera/adapter.go | Added DecodeCascadeMetadata & VerifySignature methods, updated toSdkAction mapping |
Comments suppressed due to low confidence (3)
sdk/task/cascade.go:38
- [nitpick] The
actionIdfield onCascadeTaskappears redundant sinceBaseTask.ActionIDis used elsewhere; consider removing the lowercaseactionIdfield to reduce confusion.
func NewCascadeTask(base BaseTask, filePath string, actionId string) *CascadeTask {
sdk/task/cache.go:134
- There are no existing tests covering
UpdateTxHashor the newActionID/TxHashfields inTaskEntry; add unit tests to ensure these new behaviors are validated.
func (tc *TaskCache) UpdateTxHash(ctx context.Context, taskID string, txHash string) bool {
sdk/adapters/lumera/adapter.go:12
- The code uses
fmt.Errorfin new methods butfmtis not imported; addimport "fmt"to avoid compile errors.
import (
Comment on lines
+58
to
+61
| base64EnTcketDataHash := cascadeMetaData.DataHash | ||
|
|
||
| // Decode the data hash from base64 to raw bytes | ||
| dataHashBytes, err := base64.StdEncoding.DecodeString(base64EnTcketDataHash) |
There was a problem hiding this comment.
The variable name base64EnTcketDataHash is misspelled and unclear; consider renaming it to base64EncodedDataHash for clarity.
Suggested change
| base64EnTcketDataHash := cascadeMetaData.DataHash | |
| // Decode the data hash from base64 to raw bytes | |
| dataHashBytes, err := base64.StdEncoding.DecodeString(base64EnTcketDataHash) | |
| base64EncodedDataHash := cascadeMetaData.DataHash | |
| // Decode the data hash from base64 to raw bytes | |
| dataHashBytes, err := base64.StdEncoding.DecodeString(base64EncodedDataHash) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.