Skip to content
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

[ADHOC] chore(evm): set up deploy logic for LimitedSignerValidator #404

Merged
merged 4 commits into from
Feb 10, 2025

Conversation

topocount
Copy link
Collaborator

@topocount topocount commented Feb 10, 2025

This PR deploys LimitedSignerValidator for use as a part of the protocol.
This will hard-cap all incentive claims, regardless of how many valid actions
the claimant submits.

Testing

Ensured that this deployment works on private forks of Base mainnet and also sepolia

@topocount topocount requested a review from Quazia February 10, 2025 16:58
Copy link

changeset-bot bot commented Feb 10, 2025

🦋 Changeset detected

Latest commit: f040e96

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@boostxyz/evm Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions bot added the EVM label Feb 10, 2025
@Quazia
Copy link
Member

Quazia commented Feb 10, 2025

Warnings
⚠️

By using the [ADHOC] title prefix, you are bypassing best practice protections.

⚠️

Are you sure you want to be submitting a change without including a changeset? If you're just changing docs or tests, you probably don't need to. See the publishing section of the README for more info.

Generated by 🚫 dangerJS against 2582c14

@topocount topocount changed the title [ADHOC]chore(evm): deploy LimitedSignerValidator [ADHOC] chore(evm): set up deploy logic for LimitedSignerValidator Feb 10, 2025
@Quazia
Copy link
Member

Quazia commented Feb 10, 2025

Warnings
⚠️

By using the [ADHOC] title prefix, you are bypassing best practice protections.

⚠️

Are you sure you want to be submitting a change without including a changeset? If you're just changing docs or tests, you probably don't need to. See the publishing section of the README for more info.

Generated by 🚫 dangerJS against 2582c14

1 similar comment
@Quazia
Copy link
Member

Quazia commented Feb 10, 2025

Warnings
⚠️

By using the [ADHOC] title prefix, you are bypassing best practice protections.

⚠️

Are you sure you want to be submitting a change without including a changeset? If you're just changing docs or tests, you probably don't need to. See the publishing section of the README for more info.

Generated by 🚫 dangerJS against 2582c14

Copy link
Member

@Quazia Quazia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added the broadcasts @topocount You want me to do the SDK changes in this PR or spin up a second one for that part?

`LimitedSignerValidator` hard caps the claim quantity for a given incentive
regardless of the validity of individual action requests presented to the signer.

Most incentives hard-cap each address to one claim, but `ERC20PeggedVariableCriteriaIncentive`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe there are other incentives that don't hard-cap this, may be worth checking and adding.

@Quazia
Copy link
Member

Quazia commented Feb 10, 2025

Warnings
⚠️

By using the [ADHOC] title prefix, you are bypassing best practice protections.

⚠️

Are you sure you want to be submitting a change without including a changeset? If you're just changing docs or tests, you probably don't need to. See the publishing section of the README for more info.

Generated by 🚫 dangerJS against e78666d

@topocount
Copy link
Collaborator Author

I added the broadcasts @topocount You want me to do the SDK changes in this PR or spin up a second one for that part?

@Quazia Let's do those in a second PR. The SDK class exists for this, but it'd be good to add it as a default where necessary.

@Quazia
Copy link
Member

Quazia commented Feb 10, 2025

Warnings
⚠️

By using the [ADHOC] title prefix, you are bypassing best practice protections.

⚠️

Are you sure you want to be submitting a change without including a changeset? If you're just changing docs or tests, you probably don't need to. See the publishing section of the README for more info.

Generated by 🚫 dangerJS against 33eb72a

Comment on lines 97 to 98
does not, by design.
and `ERC20VariableIncentive` do not, by design. `ERC1155Incentive` catalogs
individual claims by transaction hash, as opposed to claimant.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Quazia I went through the incentives and called out all that don't track claim target.

@topocount topocount enabled auto-merge (rebase) February 10, 2025 20:39
@Quazia
Copy link
Member

Quazia commented Feb 10, 2025

Warnings
⚠️

By using the [ADHOC] title prefix, you are bypassing best practice protections.

Generated by 🚫 dangerJS against f040e96

@Quazia
Copy link
Member

Quazia commented Feb 10, 2025

@topocount Yeah it's plugging in the defaults specifically that I wanted to handle. I'll get that going right now

@topocount topocount merged commit 8c3369c into main Feb 10, 2025
6 checks passed
@topocount topocount deleted the kevin/adhoc-limited-validator-deploy branch February 10, 2025 20:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants