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

Add keepers example using function selector checkData #73

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

giovannidisiena
Copy link

This PR extends the KeepersCounter.sol contract to add example usage of function selectors as checkData.

Tested locally and on Kovan testnet using Upkeep IDs [2028, 2029, 2030].

Corresponding tests will skip if multiplierEnabled is set to "false" in helper-hardhat-config.js.

@PatrickAlphaC
Copy link
Contributor

Ooohhhh. I like the idea of using an example with a perforumData. @andrejrakic can you take a look?

@PatrickAlphaC
Copy link
Contributor

Ahhh, sorry @giovannidisiena. Want to update this and we'll give it a proper look?

@PatrickAlphaC
Copy link
Contributor

Wait.... How did @rdotterer09 approve the PR?

@giovannidisiena
Copy link
Author

Ahhh, sorry @giovannidisiena. Want to update this and we'll give it a proper look?

Not a problem @PatrickAlphaC, likewise apologies for the delay. Assume you just mean to update by rebase and resolve conflicts right?

Wait.... How did @rdotterer09 approve the PR?

Lol no idea

@rdotterer09
Copy link

Guys, I'm not a coder, or trader, just an investor, I'm slowly learning how to read this code business so I am no threat to you, please update me on the total I have in contract after conversion is completed, Thank You

@rdotterer09
Copy link

Is important for you guys to know I have some of the contract addresses on two different hard wallets, some on Arculus and some on nano X

@PatrickAlphaC
Copy link
Contributor

@rdotterer09 no worries, we found out anyone can approve PRs on github ahah.

@PatrickAlphaC
Copy link
Contributor

and @giovannidisiena, yes!

@rdotterer09
Copy link

Are you guys getting any closer to finishing this project? Just concerned about my losses in this lovely market crunch

@giovannidisiena
Copy link
Author

Cool, should be ready for review now @PatrickAlphaC 😄

@pappas999
Copy link
Collaborator

thanks @giovannidisiena ! @andrejrakic can you please review

Copy link
Contributor

@andrejrakic andrejrakic left a comment

Choose a reason for hiding this comment

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

Thanks for opening this PR, @giovannidisiena! Awesome job! However, I am not convinced that this example will be clear to developers, especially those new to Solidity. Maybe we can develop a more straightforward example that includes both checkData and performData.

@giovannidisiena
Copy link
Author

Thanks @andrejrakic! Sure, that makes complete sense. Maybe if the timestamp fulfils some condition provided by checkData then checkUpkeep can return some performData upon which to act. Did you have any ideas?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants