add tx_priority_fee argument for program deploy subcommand by Nagaprasadvr · Pull Request #144 · anza-xyz/agave

Nagaprasadvr · 2024-03-08T04:18:51Z

Issue - #21

Add Transaction priority fee argument in terms of LAMPORTS to solana program deploy subcommand

Problem

solana program deploy command could take a lot of time to finish due to program code size .Transaction execution speeds can be increased or txs can be prioritised by setting compute_unit_price (priority fee) for every tx this price is set and txs are executes faster than normal therefore making program deployment faster.
usage - solana program deploy --tx-priority-fee 1000000

Summary of Changes

solana/cli/program - added tx_priority_fee logic

Nagaprasadvr · 2024-03-08T04:22:17Z

Should we update program_v4 as well ?

tao-stones · 2024-03-11T21:51:08Z

        )
    };
+
+    initial_instructions.extend(ixs);


It could error out when making ixs above, so maybe more economic to extend ComputeBudgetInstruction::set_compute_unit_price(...) into ixs, instead of allocate set_compute_unit_price then fails on rpc call.

After reading this https://www.helius.dev/blog/priority-fees-understanding-solanas-transaction-fee-mechanics Section:Best practices , it says set_compute_limit and set_compute_units should always be added to the start of ix Vec.

So a solution for this can be i create set_compute_units and set_compute_limit instructions after ixs is prepared ?
at line no 2318

t-nelson · 2024-03-12T05:32:12Z

+                        )
+                        .arg(
+                            Arg::with_name("compute_unit_price")
+                                .long("compute-unit-price")
+                                .takes_value(true)
+                                .help(
+                                    "Set computeUnitPrice (tx priotity fee) to boost transactions resulting in faster execution speed \
+                                    [Note: priority fee is added in LAMPORTS, so enter priority fee in terms of LAMPORTS]",
+                                ),
+                        ).
+                        arg(
+                            Arg::with_name("max_retires")
+                                .long("max-retries")
+                                .takes_value(true)
+                                .help(
+                                    "Specifies the max retry count for failed transactions",
+                                ),


these should be added in separate PRs

so compute_unit_price is one pr and max_retries is another pr ?

t-nelson · 2024-03-12T05:34:11Z

+                        .arg(
+                            Arg::with_name("compute_unit_price")
+                                .long("compute-unit-price")
+                                .takes_value(true)
+                                .help(
+                                    "Set computeUnitPrice (tx priotity fee) to boost transactions resulting in faster execution speed \
+                                    [Note: priority fee is added in LAMPORTS, so enter priority fee in terms of LAMPORTS]",
+                                ),
+                        ).


we'll want to define this arg somewhere and such that it can be reused on all transaction-bearing subcommands, for consistencies sake. most likely that will be in the clap-utils dir, similarly to the offline_signing module

can we do this in a diff pr ?

ChewingGlass · 2024-03-13T21:52:38Z

FYI this seems to only put priority fees on the initial buffer creation transaction and not on all the subsequent txns (the ones that take the lion's share of the time)

Nagaprasadvr · 2024-03-14T17:49:13Z

FYI this seems to only put priority fees on the initial buffer creation transaction and not on all the subsequent txns (the ones that take the lion's share of the time)

I have added set compute limit and set compute price ixs to write messages and final messages as well

ChewingGlass · 2024-03-14T18:32:42Z

+                3 as u32 * 200_000u32,
+            ));
+            ixs.push(ComputeBudgetInstruction::set_compute_unit_price(
+                0u64.mul((10 as u64).pow(6)),


Will always be 0?

Suggested change

0u64.mul((10 as u64).pow(6)),

compute_unit_price.mul((10 as u64).pow(6)),

Will always be 0?

Oh my bad , that is wrong it should be compute_unit_price

ChewingGlass · 2024-03-14T18:49:42Z

The arg should be in terms of microlamports, otherwise the min you can pay is 0.000605 SOL per write tx. Which gets pricy.

Nagaprasadvr · 2024-03-15T01:07:27Z

The arg should be in terms of microlamports, otherwise the min you can pay is 0.000605 SOL per write tx. Which gets pricy.

Will change to micro lamports

Nagaprasadvr · 2024-03-15T04:25:35Z

The arg should be in terms of microlamports, otherwise the min you can pay is 0.000605 SOL per write tx. Which gets pricy.

resolved

t-nelson · 2024-03-15T05:13:46Z

-                        ),
+                        )
+                        .arg(
+                            Arg::with_name("compute_unit_price")


prefer the pre-defined Arg from clap-utils

agave/clap-utils/src/compute_unit_price.rs

Lines 9 to 15 in a2579d4

pub fn compute_unit_price_arg<'a, 'b>() -> Arg<'a, 'b> {

Arg::with_name(COMPUTE_UNIT_PRICE_ARG.name)

.long(COMPUTE_UNIT_PRICE_ARG.long)

.takes_value(true)

.value_name("COMPUTE-UNIT-PRICE")

.help(COMPUTE_UNIT_PRICE_ARG.help)

}

t-nelson · 2024-03-15T05:18:00Z

+    ixs_len: usize,
+) {
+    if let Some(compute_unit_price) = compute_unit_price {
+        println!("ixs_len: {}", ixs_len);


remove this debug print

t-nelson · 2024-03-15T05:19:23Z

+    if let Some(compute_unit_price) = compute_unit_price {
+        println!("ixs_len: {}", ixs_len);
+        ixs.push(ComputeBudgetInstruction::set_compute_unit_limit(
+            ((ixs_len + 2) as u32) * 200_000u32,


most likely this is a drastic over-reservation which may reduce the likelihood of the transaction being included toward the end of a block

Can u elaborate what is wrong here exactly is it setting more cu than required?

i have also read that setting compute_unit_limit is optional when setting compute_unit_price wyt?

It is optional, but it's recommended to set a limit because the default limit is 200k * the number of instructions in the transaction. The transaction priority fee is calculated by multiplying the micro-lamport fee (per compute unit) by the transaction's compute unit limit. So if you set the compute unit too high or if you don't specify one and use the default limit, you will end up overpaying priority fees.. in other words, wasting tokens.

This set_compute_budget_ixs_if_needed should probably accept compute_unit_limit argument explicitly and then each caller can calculate the exact compute unit limit needed for the passed ixs. Then this function can add 300 compute units to the limit for the two additional compute budget program instructions. The default compute units charged per core program instruction are as follows:

System program: 150

Compute budget program: 150

Upgradeable BPF loader program: 2370

will add this logic ty

so we need to check each ix in ix vec and calculate compute_units required in this case right?

Yes each ix has to be counted. I already explained this above and gave my suggestion for how to implement the fix.

Nagaprasadvr · 2024-03-16T12:11:08Z

found a workaround for this
@jstarry

i have added 2000 cu when program is neither system_program or bpf_upgradable_loader

…and and implement the logic

…inal message

Nagaprasadvr · 2024-03-19T04:44:53Z

please look at #312 ! apologies for closing this pr , please add your comments there

mergify Bot added community need:merge-assist labels Mar 8, 2024

mergify Bot requested a review from a team March 8, 2024 04:19

Nagaprasadvr marked this pull request as draft March 8, 2024 04:21

tao-stones reviewed Mar 8, 2024

View reviewed changes

Comment thread cli/src/program.rs Outdated

Nagaprasadvr marked this pull request as ready for review March 9, 2024 03:44

Nagaprasadvr requested a review from tao-stones March 9, 2024 03:45

tao-stones reviewed Mar 11, 2024

View reviewed changes

t-nelson reviewed Mar 12, 2024

View reviewed changes

Nagaprasadvr changed the title ~~add tx_priority_fee and max_retries arguments for program deploy subcommand~~ add tx_priority_fee arguments for program deploy subcommand Mar 14, 2024

Nagaprasadvr changed the title ~~add tx_priority_fee arguments for program deploy subcommand~~ add tx_priority_fee argument for program deploy subcommand Mar 14, 2024

Nagaprasadvr requested review from t-nelson and tao-stones March 14, 2024 17:56

ChewingGlass reviewed Mar 14, 2024

View reviewed changes

mikemaccana mentioned this pull request Mar 14, 2024

Unable to deploy, max retries... solana-foundation/anchor#2836

Closed

Nagaprasadvr mentioned this pull request Mar 15, 2024

Implement prioritization fees for program deploy using cli #239

Closed

Nagaprasadvr requested a review from ChewingGlass March 15, 2024 04:18

t-nelson reviewed Mar 15, 2024

View reviewed changes

anonymous and others added 5 commits March 18, 2024 07:29

add tx_priority_fee and max_retries arguments for program deploy comm…

c86e11c

…and and implement the logic

change tx_priority_fee to compute_unit_price

145990e

change set-compute-price help description

d7218b7

remove max retries from this pr

0e37d7e

remove max retries argument

d256ff0

Nagaprasadvr added 6 commits March 18, 2024 07:29

set compute limits along with setting compute unit price

9c9f925

set compute_limit and set compute_unit_price for write messages and f…

05de101

…inal message

reduce code while adding compute budget ixs

aa25895

remove logging

013b672

use compute_unit_price_arg

b9f44d2

set compute units based on programs

76ab130

Nagaprasadvr requested review from jstarry and t-nelson March 18, 2024 02:00

Nagaprasadvr closed this by deleting the head repository Mar 19, 2024

Nagaprasadvr mentioned this pull request Mar 19, 2024

add tx_priority_fee argument for program deploy subcommand #312

Closed

	0u64.mul((10 as u64).pow(6)),
	compute_unit_price.mul((10 as u64).pow(6)),

	pub fn compute_unit_price_arg<'a, 'b>() -> Arg<'a, 'b> {
	Arg::with_name(COMPUTE_UNIT_PRICE_ARG.name)
	.long(COMPUTE_UNIT_PRICE_ARG.long)
	.takes_value(true)
	.value_name("COMPUTE-UNIT-PRICE")
	.help(COMPUTE_UNIT_PRICE_ARG.help)
	}

Conversation

Nagaprasadvr commented Mar 8, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Summary of Changes

Uh oh!

Nagaprasadvr commented Mar 8, 2024

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Nagaprasadvr Mar 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Nagaprasadvr Mar 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ChewingGlass commented Mar 13, 2024

Uh oh!

Nagaprasadvr commented Mar 14, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Nagaprasadvr Mar 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ChewingGlass commented Mar 14, 2024

Uh oh!

Nagaprasadvr commented Mar 15, 2024

Uh oh!

Nagaprasadvr commented Mar 15, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Nagaprasadvr Mar 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Nagaprasadvr commented Mar 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Nagaprasadvr commented Mar 19, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Nagaprasadvr commented Mar 8, 2024 •

edited

Loading

Nagaprasadvr Mar 13, 2024 •

edited

Loading

Nagaprasadvr Mar 13, 2024 •

edited

Loading

Nagaprasadvr Mar 15, 2024 •

edited

Loading

Nagaprasadvr Mar 15, 2024 •

edited

Loading

Nagaprasadvr commented Mar 16, 2024 •

edited

Loading