Skip to content

Conversation

awisniew207
Copy link
Contributor

  • Gives relayer users the ability to remove payees in the DB
  • Added a test to ensure the functionality of the new code

Copy link
Contributor

@glitch003 glitch003 left a comment

Choose a reason for hiding this comment

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

looks great, thx! question - did we really need to increase the timeout to 60s? like we're making txns on chronicle, which should be instant? i'm okay with keeping it at 60s, that's fine, but just curious if you saw problems at 30s? i know for example, the ethers default polling time is 7s, so we may want to set that to something like 1s since we have a fast chain.

here's how to change the polling interval, for reference, if you wanna give that a shot in another PR. feel free to merge this one tho - looks great! https://www.notion.so/litprotocol/Blog-post-Optimizing-ethers-for-low-latency-transactions-on-rollups-b65db9afaaf84c8ab02f1178941409e3?pvs=4#c727307243c8482c839e9e5534931e10

@awisniew207
Copy link
Contributor Author

looks great, thx! question - did we really need to increase the timeout to 60s? like we're making txns on chronicle, which should be instant? i'm okay with keeping it at 60s, that's fine, but just curious if you saw problems at 30s? i know for example, the ethers default polling time is 7s, so we may want to set that to something like 1s since we have a fast chain.

here's how to change the polling interval, for reference, if you wanna give that a shot in another PR. feel free to merge this one tho - looks great! https://www.notion.so/litprotocol/Blog-post-Optimizing-ethers-for-low-latency-transactions-on-rollups-b65db9afaaf84c8ab02f1178941409e3?pvs=4#c727307243c8482c839e9e5534931e10

image
I agree, it was only done so that the CI Actions passed. Not sure why, possible race condition between tests?

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.

2 participants