-
Notifications
You must be signed in to change notification settings - Fork 81
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
feat: add advanced contract condition for gated-content #555
feat: add advanced contract condition for gated-content #555
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
Are all these changes intentional?
The peer dependencies are meant to be satisfied by the @lens-protocol/client
(and later by the @lens-protocol/react
) packages as this package is not a standalone public package.
value: ':userAddress', | ||
...overrides, | ||
}; | ||
} |
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.
I see this helper but not use of it, was a test suite meant to be checked-in?
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.
yeah tests are still WIP, will push them soon
packages/gated-content/src/conditions/advanced-contract-condition.ts
Outdated
Show resolved
Hide resolved
packages/gated-content/src/conditions/advanced-contract-condition.ts
Outdated
Show resolved
Hide resolved
packages/gated-content/src/conditions/advanced-contract-condition.ts
Outdated
Show resolved
Hide resolved
); | ||
|
||
// for bool, array, tuple results we only allow equal/not equal | ||
if (['bool', 'array', 'tuple'].includes(fn.outputs[0].baseType)) { |
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.
I think this should be codified in the type and validation within the @lens-protocol/metadata
package.
contract: toRawNetworkAddress(gqlCondition.contract), | ||
abi: gqlCondition.abi, | ||
functionName: gqlCondition.functionName, | ||
params: gqlCondition.params as string[], |
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 this because of a readonly?
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.
yeah, open to suggestions to clean this up
d1e3dde
to
24d2c0e
Compare
ad4cc0a
to
a9b46b5
Compare
@@ -194,6 +196,19 @@ function toRawSimpleCondition(gqlCondition: gql.ThirdTierCondition): raw.SimpleC | |||
follow: raw.toProfileId(gqlCondition.follow), | |||
}); | |||
|
|||
case 'AdvancedContractCondition': | |||
return { |
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.
use the new raw.advancedContractCondition
builder, it should reduce the boilerplate code here. Don't know if it can solve the readonly. Minor thing anyway.
@@ -45,6 +45,9 @@ | |||
}, | |||
"license": "MIT", | |||
"dependencies": { | |||
"@ethersproject/abi": "^5.7.0", | |||
"@ethersproject/address": "^5.7.0", | |||
"@ethersproject/bignumber": "^5.7.0", |
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.
Unless something is changed these should stay as peer dependency only. Maybe there is an issue somewhere else, can you please clarify what made you make this change?
Merged here: #707 |
No description provided.