Feat: Support estimate_fee_bulk API #461

irisdv · 2022-12-16T14:11:37Z

Motivation and Resolution

Feat #388
Adds support for estimate_fee_bulk API introduced in Cairo 0.10.2

Usage related changes

Adds a new function estimateFeeBulk in Account class, taking in arguments an array of transactions and returns an array of EstimateFeeResponse. Implemented only with sequencer, calling estimateFeeBulk with RPC will throw an error.

Checklist:

Performed a self-review of the code
Rebased to the last commit of the target branch (or merged it into my branch)
Linked the issues which this PR resolves
Documented the changes
Updated the docs (www)
Updated the tests
All tests are passing

netlify · 2022-12-16T14:11:41Z

✅ Deploy Preview for starknetjs canceled.

Name	Link
🔨 Latest commit	`8e0f3da`
🔍 Latest deploy log	https://app.netlify.com/sites/starknetjs/deploys/63bc4e946a4df50008c25f4d

ivpavici · 2022-12-20T18:41:35Z

tests fail because we need to update devnet from 0.4.0 to 0.4.2

penovicp · 2022-12-21T07:01:38Z

tests fail because we need to update devnet from 0.4.0 to 0.4.2

Just to clarify, this means bumping shardlabs/starknet-devnet:0.4.0-seed0 in the .github/workflows files so it can be done as part of this PR.

yoga-braavos · 2022-12-21T08:27:09Z

__tests__/account.test.ts

+  test('estimate fee bulk', async () => {
+    const res = await account.estimateFeeBulk([
+      {
+        type: 'DECLARE',


@irisdv I suggest that on top of this "generic" test, we add a test for the main scenario this bulk endpoint caters which is:
Txn 1: DEPLOY_ACCOUNT
Txn 2: Multi-call (of INVOKE-s) on the account deployed account from Txn 1

When adding this test I realised I have an issue when estimating fees for DEPLOY_ACCOUNT. I have this error when I’m trying to use the function estimateAccountDeployFee on its own : Entry point 0x36fcbf06cd96843058359e1a75928beacfac10727dab22a3972f0af8aa92895 not found in contract with class hash 0x3fcbf77b28c96f4f2fb5bd2d176ab083a12a5e123adeb0de955d7ee228c9854

Is there a specific way to make it work ? My understanding is that to deploy a new account I need to declare it, send eth to the address then deploy it.

The 0x36fcbf06cd96843058359e1a75928beacfac10727dab22a3972f0af8aa92895 selector is __validate_deploy__ meaning that the contract you tried to deploy does not implement this endpoint and thus DEPLOY_ACCOUNT fails. In order for it to work, you need to implement/use a contract that have the __validate_deploy__ entry-point - See example in OpenZeppelin's account contract:
https://github.com/OpenZeppelin/cairo-contracts/blob/e0e07555b319d8131a35259b55269ba7b19e744e/src/openzeppelin/account/presets/Account.cairo#L112

Thanks, I updated the version of Account.json in the repo and added this testcase.

yoga-braavos · 2022-12-21T08:39:16Z

src/types/lib.ts

@@ -77,6 +77,19 @@ export type InvocationsDetails = {

 export type InvocationsDetailsWithNonce = InvocationsDetails & { nonce: BigNumberish };

+export type TransactionBulk = { type: TransactionType } & (


@irisdv why are you calling it "Bulk" if it's a single object? I would move the wrapping in an Array to the type to justify its "bulk" naming and change function signature to accept TransactionBulk rather than Array<TransactionBulk>

yoga-braavos · 2022-12-21T08:39:35Z

src/types/lib.ts

+  | DeployAccountContractPayload
+);
+
+export type InvocationBulk = { type: TransactionType } & (


@irisdv same comment re "Bulk" semantics as above

yoga-braavos · 2022-12-21T08:44:25Z

src/account/default.ts

+    const version = toBN(feeTransactionVersion);
+    const chainId = await this.getChainId();
+
+    const invocations: any = await Promise.all(


@irisdv this is a big code dup with the specialized estimate*Fee functions, please consider some refactoring so each flavor is managed in a single place in the code and the wrapping functions use that place instead of implementing it on their own

dhruvkelawala · 2023-01-04T11:30:31Z

Let's fix these tests and include it in next v4 release please @irisdv

dhruvkelawala · 2023-01-06T10:17:23Z

Is this PR ready for review?

irisdv · 2023-01-06T10:22:09Z

Yes it's ready for review, I fixed the tests

src/account/default.ts

src/account/interface.ts

src/provider/rpc.ts

src/provider/sequencer.ts

src/types/api/sequencer.ts

src/types/lib.ts

dhruvkelawala · 2023-01-09T16:08:25Z

src/provider/sequencer.ts

+    invocations: InvocationBulk,
+    blockIdentifier: BlockIdentifier = this.blockIdentifier
+  ): Promise<EstimateFeeResponseBulk> {
+    const params: any = [].concat(invocations as []).map((invocation: any) => {


Suggested change

const params: any = [].concat(invocations as []).map((invocation: any) => {

const params: Sequencer.EstimateFeeRequestBulk = invocations.map((invocation) => {

Just a small change, we can merge after this

dhruvkelawala · 2023-01-09T17:27:41Z

Thanks for your help @irisdv . Appreciate this!

github-actions · 2023-01-09T19:52:40Z

🎉 This PR is included in version 4.19.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

github-actions · 2023-01-13T13:56:16Z

🎉 This PR is included in version 5.0.0-beta.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

irisdv added 2 commits December 12, 2022 14:59

feat: update sequencer api

a12f99e

feat: add function estimateInvokeFeeBulk

e716a18

irisdv added 3 commits December 20, 2022 14:18

feat: update estimateFeeBulk

5ee94f7

Merge branch 'develop' into feat/add_estimate_fee_bulk

9093b41

feat: update estimateFeeBulk function & doc

ce20139

irisdv changed the title ~~[WIP] Feat: Support estimate_fee_bulk API~~ Feat: Support estimate_fee_bulk API Dec 20, 2022

ivpavici requested review from dhruvkelawala, janek26 and penovicp December 20, 2022 18:38

yoga-braavos reviewed Dec 21, 2022

View reviewed changes

irisdv added 3 commits December 22, 2022 08:30

refactor: estimate fee bulk function

2c7a4d8

ci: update devnet from 0.4.0 to 0.4.2

7b46d1b

test: add test case estimate fee bulk

319b286

irisdv added 3 commits January 5, 2023 11:56

test: run estimate fee bulk tests only for sequencer

b79b192

fix: merge develop into feat/add_estimate_fee_bulk

88e76af

test: update account class hash in rpcProvider tests

792b2dc

dhruvkelawala requested changes Jan 6, 2023

View reviewed changes

fix: update types & add separate responseParser for estimateFeeBulk

52110b2

dhruvkelawala requested changes Jan 9, 2023

View reviewed changes

fix: update getEstimateFeeBulk function

4b22c82

Merge branch 'develop' into feat/add_estimate_fee_bulk

8e0f3da

dhruvkelawala approved these changes Jan 9, 2023

View reviewed changes

dhruvkelawala merged commit 1352e78 into starknet-io:develop Jan 9, 2023

github-actions bot added the released on @next label Jan 9, 2023

github-actions bot added the released on @beta label Jan 13, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat: Support estimate_fee_bulk API #461

Feat: Support estimate_fee_bulk API #461

irisdv commented Dec 16, 2022 •

edited

Loading

netlify bot commented Dec 16, 2022 •

edited

Loading

ivpavici commented Dec 20, 2022

penovicp commented Dec 21, 2022

yoga-braavos Dec 21, 2022 •

edited

Loading

irisdv Dec 22, 2022

yoga-braavos Dec 22, 2022

irisdv Dec 22, 2022

yoga-braavos Dec 21, 2022 •

edited

Loading

yoga-braavos Dec 21, 2022

yoga-braavos Dec 21, 2022

dhruvkelawala commented Jan 4, 2023

dhruvkelawala commented Jan 6, 2023

irisdv commented Jan 6, 2023

dhruvkelawala Jan 9, 2023

dhruvkelawala Jan 9, 2023

dhruvkelawala commented Jan 9, 2023

github-actions bot commented Jan 9, 2023

github-actions bot commented Jan 13, 2023

		@@ -77,6 +77,19 @@ export type InvocationsDetails = {

		export type InvocationsDetailsWithNonce = InvocationsDetails & { nonce: BigNumberish };

		export type TransactionBulk = { type: TransactionType } & (

	const params: any = [].concat(invocations as []).map((invocation: any) => {
	const params: Sequencer.EstimateFeeRequestBulk = invocations.map((invocation) => {

Feat: Support estimate_fee_bulk API #461

Feat: Support estimate_fee_bulk API #461

Conversation

irisdv commented Dec 16, 2022 • edited Loading

Motivation and Resolution

Usage related changes

Checklist:

netlify bot commented Dec 16, 2022 • edited Loading

✅ Deploy Preview for starknetjs canceled.

ivpavici commented Dec 20, 2022

penovicp commented Dec 21, 2022

yoga-braavos Dec 21, 2022 • edited Loading

Choose a reason for hiding this comment

irisdv Dec 22, 2022

Choose a reason for hiding this comment

yoga-braavos Dec 22, 2022

Choose a reason for hiding this comment

irisdv Dec 22, 2022

Choose a reason for hiding this comment

yoga-braavos Dec 21, 2022 • edited Loading

Choose a reason for hiding this comment

yoga-braavos Dec 21, 2022

Choose a reason for hiding this comment

yoga-braavos Dec 21, 2022

Choose a reason for hiding this comment

dhruvkelawala commented Jan 4, 2023

dhruvkelawala commented Jan 6, 2023

irisdv commented Jan 6, 2023

dhruvkelawala Jan 9, 2023

Choose a reason for hiding this comment

dhruvkelawala Jan 9, 2023

Choose a reason for hiding this comment

dhruvkelawala commented Jan 9, 2023

github-actions bot commented Jan 9, 2023

github-actions bot commented Jan 13, 2023

irisdv commented Dec 16, 2022 •

edited

Loading

netlify bot commented Dec 16, 2022 •

edited

Loading

yoga-braavos Dec 21, 2022 •

edited

Loading

yoga-braavos Dec 21, 2022 •

edited

Loading