[BRI-1784] Validate low level declaration args #142

phanpak · 2023-12-22T08:34:49Z

No description provided.

MatiasOS

Please, change .string() values to hex validation

MatiasOS · 2023-12-27T15:29:11Z

src/intents/schemas/lowLevel.ts

+});
+
+const SegmentStructSchema = joi.object({
+  data: joi.string().required(),


data isn't required

MatiasOS · 2023-12-27T15:36:01Z

src/intents/schemas/lowLevel.ts

+  value: joi.string().required()
+});
+
+const SegmentParamValueSchema = joi.alternatives().try(


Please, add some test with wrong parameters that fails at this level

agree on this - and especially we should check that the error messages are meaningful. We had this issue with DSL schema where all Joi errors were from the top level and did not say which parameter the error happened at

mikec

would like to see tests for specific errors and their messages before merging

mikec · 2023-12-27T16:16:15Z

src/intents/schemas/lowLevel.ts

@@ -0,0 +1,117 @@
+import { segmentFunctionNames, signatureTypes, tokenStandards } from "./constants";


we have been calling it "low level" definition, but we should come up with a better name for this.

Maybe we call it "functional"? so we would have DSL Definition level, and functional level

mikec · 2023-12-27T16:17:42Z

src/intents/schemas/lowLevel.ts

+  value: joi.string().required()
+});
+
+const SegmentParamValueSchema = joi.alternatives().try(


agree on this - and especially we should check that the error messages are meaningful. We had this issue with DSL schema where all Joi errors were from the top level and did not say which parameter the error happened at

mikec · 2023-12-27T16:18:20Z

test/local/fullSchema.test.ts

+describe('DeclarationArgsSchema', () => {
+  it('should validate a valid declaration', () => {
+    const { error } = DeclarationArgsSchema.validate(declarationArgs);
+    console.log("@@@@@@@@@@@@@@@@@@@", error);


debugging? might want to remove

phanpak · 2023-12-28T17:31:45Z

src/intents/DSL/actions/blockIntervalDutchAuctionSwapAction.ts

        signer: owner,
-        tokenIn: tokenIn as SegmentParamValue,
-        tokenOut: tokenOut as SegmentParamValue,
+        tokenIn: { address: tokenIn.address } as SegmentParamValue,


@mikec can you tell me whether it's correct? The thing is tokenIn and tokenOut contain values that are irrelevant for segments.

I think this will work because we are not supporting other values for token struct to be set with DSL. The additional struct values were for ERC721 (NFT's) which we're not currently dealing with. If tests for DSL are passing, this should be ok.

mikec · 2023-12-28T17:52:02Z

src/intents/DSL/actions/blockIntervalDutchAuctionSwapAction.ts

        signer: owner,
-        tokenIn: tokenIn as SegmentParamValue,
-        tokenOut: tokenOut as SegmentParamValue,
+        tokenIn: { address: tokenIn.address } as SegmentParamValue,


I think this will work because we are not supporting other values for token struct to be set with DSL. The additional struct values were for ERC721 (NFT's) which we're not currently dealing with. If tests for DSL are passing, this should be ok.

mikec · 2023-12-28T17:53:15Z

test/local/schemas/functional.test.ts

+
+
+const declarationArgs = {
+		'data': '0x4f899fb2000000000000000000000000000000000000000000000000000000000000004000000000000000000000000000000000000000000000000000000000000007c000000000000000000000000079f147176824196789f07dfc2d21e100ee1ae13800000000000000000000000000000000000000000000000000000000000000800000000000000000000000000000000000000000000000000000000000000740000000000000000000000000000000000000000000000000000000000000076000000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000040000000000000000000000000000000000000000000000000000000000000080000000000000000000000000000000000000000000000000000000000000014000000000000000000000000000000000000000000000000000000000000001e00000000000000000000000000000000000000000000000000000000000000320000000000000000000000000000000000000000000000000000000000000004000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000044a9aa5f7e0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000004000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000024502929ff00000000000000000000000000000000000000000000000000009a761dfdf100000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000040000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000c42b2fc43a0000000000000000000000003b28d6ee052b65ed4d5230c1b2a9abaef031c648000000000000000000000000000000000000000000000000000000000000006000000000000000000000000000000000000003e8000000000000000000000000000000000000000000000000000000000000000000000000000000000000004000000000000000000000000088e6a0c2ddd26feeb64f039a2c41296fcb3f564000000000000000000000000000000000000000000000000000000000000003e8000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000040000000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000000000000000000000000000000000002a449d9a2180000000000000000000000003b28d6ee052b65ed4d5230c1b2a9abaef031c64800000000000000000000000000000000000000000000000000000000000002200000000000000000000000006399ae010188f36e469fb6e62c859ddfc558328a0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000dac17f958d2ee523a2206206994597c13d831ec70000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000a0b86991c6218b36c1d19d4a2e9eb0ce3606eb4800000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000566d3e80000000000000000000000000000000000000000000000000000000000000271000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000280000000000000000000000000000000000000000000000000000000000000004000000000000000000000000088e6a0c2ddd26feeb64f039a2c41296fcb3f564000000000000000000000000000000000000000000000000000000000000003e800000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000',


data should not be needed. if you remove data from the object, will it still work?

yep still working

mikec · 2023-12-28T17:54:51Z

test/local/schemas/functional.test.ts

+    // @ts-ignore
+    value.intents[0].segments[0].params = { index: '0' }; // missing value
+    const { error } = FunctionalSchema.validate(value);
+    expect(error.message).to.match(/params.value.*is required/);


does this error tell me which missing intent/segment/param is required?

True. I made the assertion more specific.

MatiasOS suggested changes Dec 27, 2023

View reviewed changes

mikec requested changes Dec 27, 2023

View reviewed changes

phanpak force-pushed the BRI-1784-validation-args branch 2 times, most recently from 3ab41ea to 7adde84 Compare December 28, 2023 17:29

phanpak added 2 commits December 29, 2023 00:30

Add functional schema for declaration

c57042c

Fix tests

2c1c9a6

phanpak force-pushed the BRI-1784-validation-args branch from 7adde84 to 2c1c9a6 Compare December 28, 2023 17:30

phanpak commented Dec 28, 2023

View reviewed changes

mikec requested changes Dec 28, 2023

View reviewed changes

phanpak added 2 commits December 29, 2023 01:28

Improve test

69e5529

Remove unnecessary data

7c13433

mikec approved these changes Jan 12, 2024

View reviewed changes

phanpak added 3 commits January 17, 2024 00:18

Revert changes due to SegmentFunctionName

c1592e4

Bump types

ddbab0f

change some to ethereumAddress

9599f0d

MatiasOS approved these changes Jan 16, 2024

View reviewed changes

Bump

ff2558e

		@@ -0,0 +1,117 @@
		import { segmentFunctionNames, signatureTypes, tokenStandards } from "./constants";



		const declarationArgs = {
		'data': '0x4f899fb2000000000000000000000000000000000000000000000000000000000000004000000000000000000000000000000000000000000000000000000000000007c000000000000000000000000079f147176824196789f07dfc2d21e100ee1ae13800000000000000000000000000000000000000000000000000000000000000800000000000000000000000000000000000000000000000000000000000000740000000000000000000000000000000000000000000000000000000000000076000000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000040000000000000000000000000000000000000000000000000000000000000080000000000000000000000000000000000000000000000000000000000000014000000000000000000000000000000000000000000000000000000000000001e00000000000000000000000000000000000000000000000000000000000000320000000000000000000000000000000000000000000000000000000000000004000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000044a9aa5f7e0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000004000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000024502929ff00000000000000000000000000000000000000000000000000009a761dfdf100000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000040000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000c42b2fc43a0000000000000000000000003b28d6ee052b65ed4d5230c1b2a9abaef031c648000000000000000000000000000000000000000000000000000000000000006000000000000000000000000000000000000003e8000000000000000000000000000000000000000000000000000000000000000000000000000000000000004000000000000000000000000088e6a0c2ddd26feeb64f039a2c41296fcb3f564000000000000000000000000000000000000000000000000000000000000003e8000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000040000000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000000000000000000000000000000000002a449d9a2180000000000000000000000003b28d6ee052b65ed4d5230c1b2a9abaef031c64800000000000000000000000000000000000000000000000000000000000002200000000000000000000000006399ae010188f36e469fb6e62c859ddfc558328a0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000dac17f958d2ee523a2206206994597c13d831ec70000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000a0b86991c6218b36c1d19d4a2e9eb0ce3606eb4800000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000566d3e80000000000000000000000000000000000000000000000000000000000000271000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000280000000000000000000000000000000000000000000000000000000000000004000000000000000000000000088e6a0c2ddd26feeb64f039a2c41296fcb3f564000000000000000000000000000000000000000000000000000000000000003e800000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000',

[BRI-1784] Validate low level declaration args #142

Are you sure you want to change the base?

[BRI-1784] Validate low level declaration args #142

Uh oh!

Conversation

phanpak commented Dec 22, 2023

Uh oh!

MatiasOS left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mikec left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants