-
Notifications
You must be signed in to change notification settings - Fork 295
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
chore: use L1 Tx Utils #10759
chore: use L1 Tx Utils #10759
Conversation
const args = [to.toString(), amount, claimSecretHash.toString()] as const; | ||
const { request } = await this.portal.simulate.depositToAztecPublic(args); | ||
|
||
const txReceipt = await this.l1TxUtils.sendAndMonitorTransaction( | ||
{ | ||
to: request.address!, | ||
data: encodeFunctionData({ | ||
abi: this.portal.abi, | ||
functionName: 'depositToAztecPublic', | ||
args, | ||
}), | ||
}, | ||
{ | ||
fixedGas: request.gas, | ||
}, | ||
); |
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.
Do we need to call simulate
here if the tx utils already call estimateGas
under the hood, which should throw (emphasis on should, we need to verify that) if the tx would revert? Also, note that simulate
does not seem to perform a gas estimation as it's just mapped to call
, and the gas
returned seems to be whatever was originally set and not the actual gas used (see here).
// Call function on L1 contract to consume the message | ||
const { request } = await this.portal.simulate.withdraw(withdrawArgs); |
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.
Same as above
// Send transactions first | ||
const [mintTxRes, approveTxRes] = await Promise.all([ | ||
l1TxUtils.sendTransaction(mintRequest), | ||
l1TxUtils.sendTransaction(approveRequest), | ||
]); | ||
|
||
// Monitor transactions in parallel | ||
await Promise.all([ | ||
l1TxUtils.monitorTransaction(mintRequest, mintTxRes.txHash, { gasLimit: mintTxRes.gasLimit }), | ||
l1TxUtils.monitorTransaction(approveRequest, approveTxRes.txHash, { gasLimit: approveTxRes.gasLimit }), | ||
]); |
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.
Curious: why not a single Promise.all
with sendAndMonitor
?
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 feel like I had a reason and now realizing that it was probably invalid since we're sending these 2 at the same time anyway
Changes to circuit sizes
🧾 Summary (100% most significant diffs)
Full diff report 👇
|
Fixes #10464