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

support customized block number for conditional #11804

Merged
merged 1 commit into from
Jan 24, 2024

Conversation

shileiwill
Copy link
Contributor

@shileiwill shileiwill commented Jan 17, 2024

AUTO-8793

As title, support using a block number from users. This covers both checkupkeep and perform

Test plan:

Tried the following upkeep in arbitrum sepolia:

go run main.go keeper debug 67565883080818726407103359577750299084952920187169313465079851360269044647245 7817591

Both older block and latest block returned ineligible.

I couldn't find an upkeep which checkUpkeep returns true on older blocks but latest returns false. Hope to find some interesting test cases with Mike later.

Copy link
Contributor

I see that you haven't updated any CHANGELOG files. Would it make sense to do so?

@FelixFan1992
Copy link
Contributor

AUTO-8793

As title, support using a block number from users. This covers both checkupkeep and perform

Test plan:

Tried the following upkeep in arbitrum sepolia:

go run main.go keeper debug 67565883080818726407103359577750299084952920187169313465079851360269044647245 7817591

Both older block and latest block returned ineligible.

I couldn't find an upkeep which checkUpkeep returns true on older blocks but latest returns false. Hope to find some interesting test cases with Mike later.

a simple upkeep of this can be:

function checkUpkeep() { return block.number < 400000; }

Comment on lines 146 to 147
var blockNumber int64
blockNumber, err = strconv.ParseInt(args[1], 10, 64)
Copy link
Contributor

Choose a reason for hiding this comment

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

should we re-use blockNum that is declared on line 135? I believe that is the value that is used in the streams lookup section. If the users passes a blocknum for conditional upkeeps, we should use that block num for stream lookup callbacks, yeah?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They are different in my opinion.

  1. The blockNumber in L147 is from user input, and it is used only for conditional upkeeps. So users can run checkUpkeep and performUpkeep using that specific block number.
  2. The blockNumber in L135 is used in log trigger based upkeeps, and the block number is read from the transaction receipt. For log trigger upkeeps, we dont ask for block number. we have the tx hash tho.

Let me know if the above makes sense :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated~ It is a good idea to reuse the variable, hence removed the additional variable :)

@cl-sonarqube-production
Copy link

SonarQube Quality Gate

Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@shileiwill shileiwill added this pull request to the merge queue Jan 24, 2024
Merged via the queue into develop with commit 59d1c99 Jan 24, 2024
94 checks passed
@shileiwill shileiwill deleted the block-number-for-conditional branch January 24, 2024 16:56
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.

3 participants