-
Notifications
You must be signed in to change notification settings - Fork 13
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
Updated secret tests and added exectution validation #80
base: main
Are you sure you want to change the base?
Conversation
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.
@v9n could you take a look at this new test. How could we see and verify secret values in workflow, by examining the Step in Execution? This new test is meant to test the existing code on the main branch of EigenLayer-AVS, but not any new code for Secret.
options?: RequestOptions | ||
name: string, | ||
value: string, | ||
options?: SecretRequestOptions |
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.
The interface is changed to this style for Secret functions.
options?: {workflowId?: string, orgId?: string}
@@ -1,39 +1,39 @@ | |||
import * as avs_pb from "@/grpc_codegen/avs_pb"; | |||
// import * as avs_pb from "@/grpc_codegen/avs_pb"; |
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.
The Workflow, Node classes are used to serialize and format data from grpc, but for Secret we don’t have complex needs, so this file is not used yet.
data: { | ||
lang: CustomCodeLangs.JAVASCRIPT, | ||
// source: "console.log('foo bar')", | ||
source: "return {{secret.secrete_name}}", |
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.
This line doesn’t work either, due to the below error.
matchStep _Step {
nodeId: '01JKYTKYNXC192C61FNQC07RTJ',
success: false,
outputData: '',
log: 'Start execute user-input JS code at 2025-02-12 20:56:03.009607 -0800 PST m=+8309.973508042Complete Execute user-input JS code at 2025-02-12 20:56:03.009625 -0800 PST m=+8309.973525584\n' +
'error running JavaScript code:SyntaxError: SyntaxError: (anonymous): Line 1:1 Illegal return statement',
error: 'SyntaxError: SyntaxError: (anonymous): Line 1:1 Illegal return statement',
startAt: 1739422563,
endAt: 1739422563
}
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.
it should be ${{secrets.name}}
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.
return ${{secrets.name}}
type: NodeType.CustomCode, | ||
data: { | ||
lang: CustomCodeLangs.JAVASCRIPT, | ||
// source: "console.log('foo bar')", |
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.
We never validated the execution result. This is the code of the CustomNode in WorkflowTemplate, but it generates the below error
console.log
matchStep _Step {
nodeId: '01JKYTAFWBENXS1G9VKB2M7B44',
success: false,
outputData: '',
log: 'Start execute user-input JS code at 2025-02-12 20:50:52.950408 -0800 PST m=+7999.918439209Complete Execute user-input JS code at 2025-02-12 20:50:52.950545 -0800 PST m=+7999.918576584\n' +
'error running JavaScript code:ReferenceError: console is not defined at <eval>:1:1(0)',
error: 'ReferenceError: console is not defined at <eval>:1:1(0)',
startAt: 1739422252,
endAt: 1739422252
}
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.
This console issue will be fixed once we improve the js runner.
but that is also another issue i just don't want to bring up right now about secret.
for user secret, it's fine for them to log out or do anything they want.
but for ava protocol secret that we share with them, we need some safe guard. as in we won't allow console.log, we won't allow send out, only a specific list to be used (eg, only send telegram bot token to telegram server).
but that will. be for after launch, just keep in mind i do plan for the secret leakage.
const secret = new Secret({ | ||
name: `dummysecret_${getNextId()}`, | ||
describe("create secret suite", () => { | ||
it("created secret have value in workflow", async () => { |
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.
This newly added tests fails, but the rest are passing. The goal is to verify that the secret variable’s value show up at nodes, e.g. a CustomCode node.
… salt with options.authKey
@@ -218,20 +218,6 @@ describe("Authentication Tests", () => { | |||
expect(res2).toHaveProperty("factory", FACTORY_ADDRESS); | |||
}); | |||
|
|||
test("getWallets works with options.authKey", async () => { |
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.
This test has the exactly same name with the previous one, so I removed it.
I think it’s trying to test a default value salt:0, but there’s already a salt:123 created from the previous tests. We don’t have a way to reset the smart wallet response array yet.
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.
looks good
No description provided.