-
Notifications
You must be signed in to change notification settings - Fork 302
feat(fortuna): Support new Entropy v2 contract #2691
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
|
||
| /// The gas limit to use for entropy callback transactions. | ||
| pub gas_limit: u64, | ||
| pub gas_limit: u32, |
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.
u32 is the size of the field in the contract
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 in this new flow we should always make sure that the gas we set for the transaction is more than what the user requested. Now that the users set the gas limit, this is much easier and we can solve the cases where the gas used can vary a lot (from simulation time to landing time) based on other factors.
| .call() | ||
| .await?; | ||
|
|
||
| // sequence_number == 0 means the request does not exist. |
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.
why is this removed? I know that this check exists in some other places, but if we want to be safe and avoid unintended changes, we can keep it as is?
| } | ||
|
|
||
| // If replica config is present, we're running with multiple instances. | ||
| // The incoming request is assigned by modulo operation on the sequence number |
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.
Earmarked this to fix when we do the work to expose the needed types to the API for the frontend to consume
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.
this value is in the requestV2 struct you retrieved ^ to check the status
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.
Oh perfect ty
| sender, | ||
| gas_limit | ||
| ) | ||
| sqlx::query("INSERT INTO request(chain_id, network_id, provider, sequence, created_at, last_updated_at, state, request_block_number, request_tx_hash, user_random_number, sender, gas_limit) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)") |
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 query() instead of the query! macro since the compile-time checking of the queries doesn't reliably work with the local sqlite db, preventing building. Besides, since most of the queries are dynamically constructed, the compile time checking doesn't give us a guarantee they're correct anyway.
Summary
Update fortuna to set the default_gas_limit field on-chain, and then work with the new Entropy v2 interface. The main changes there are (1) replace the types of ProviderInfo and Request with their V2 variants, and (2) remove the gas_limit parameters from callback submission.
Rationale
we want to support the new version of the contract
How has this been tested?
I've tested this locally against the Entropy contract on blast-testnet (using the EntropyTester to simulate all of the various callback success / failure / out of gas conditions) and it works. I still want to test against a zk chain like Abstract, but that's pending the upgrade right now.