Skip to content
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

Allow overriding the runtime transaction account lock limit #26948

Merged
merged 2 commits into from
Aug 12, 2022

Conversation

jstarry
Copy link
Member

@jstarry jstarry commented Aug 5, 2022

Problem

The number of accounts that can be locked by each transaction is currently limited 64 accounts but is not configurable by devs who want to test transactions with more accounts.

Summary of Changes

  • Added the --transaction-account-lock-limit cli arg to solana-test-validator to override the hardcoded limit of 64
  • Allow overriding the tx account lock limit in ProgramTest

Fixes #

@jstarry jstarry added the work in progress This isn't quite right yet label Aug 5, 2022
@jstarry jstarry force-pushed the feat/override-tx-account-limit branch from adfc633 to 3754075 Compare August 6, 2022 09:37
@jstarry jstarry removed the work in progress This isn't quite right yet label Aug 6, 2022
@jstarry jstarry changed the title Add --transaction-account-lock-limit cli arg to test-validator Allow overriding the runtime transaction account lock limit Aug 6, 2022
@jstarry jstarry requested a review from brooksprumo August 7, 2022 17:04
Copy link
Contributor

@brooksprumo brooksprumo left a comment

Choose a reason for hiding this comment

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

Would it work to add a runtime_config: Arc<RuntimeConfig> field to the Accounts struct? Feels like currently there's a lot of plumbing to get access to the runtime config param. You did a nice job of making that easy to access from the bank, maybe that'll work for Accounts too?

The one downside I see is that if the bank changes a value of its runtime config, Accounts would still have the old value (due to COW). Is that a dealbreaker?

@jstarry
Copy link
Member Author

jstarry commented Aug 12, 2022

Would it work to add a runtime_config: Arc field to the Accounts struct? Feels like currently there's a lot of plumbing to get access to the runtime config param. You did a nice job of making that easy to access from the bank, maybe that'll work for Accounts too?

It could work and I was hoping to do that originally but there are a lot ways to construct Accounts and RuntimeConfig isn't always ready at each place of construction. It'd be more work to pipe the config through than it is to pass the config override to a few Accounts methods.

@jstarry jstarry merged commit 5618e9f into solana-labs:master Aug 12, 2022
@jstarry jstarry deleted the feat/override-tx-account-limit branch August 12, 2022 14:07
xiangzhu70 pushed a commit to xiangzhu70/solana that referenced this pull request Aug 17, 2022
…abs#26948)

* Add --transaction-account-lock-limit cli arg to test-validator

* Allow overriding the tx account lock limit in ProgramTest
mergify bot pushed a commit that referenced this pull request Sep 15, 2022
* Add --transaction-account-lock-limit cli arg to test-validator

* Allow overriding the tx account lock limit in ProgramTest

(cherry picked from commit 5618e9f)

# Conflicts:
#	program-test/src/lib.rs
#	runtime/src/accounts.rs
#	runtime/src/bank.rs
#	runtime/src/runtime_config.rs
#	sdk/src/transaction/sanitized.rs
#	test-validator/src/lib.rs
#	validator/src/bin/solana-test-validator.rs
mergify bot pushed a commit that referenced this pull request Sep 15, 2022
* Add --transaction-account-lock-limit cli arg to test-validator

* Allow overriding the tx account lock limit in ProgramTest

(cherry picked from commit 5618e9f)

# Conflicts:
#	program-test/src/lib.rs
#	test-validator/src/lib.rs
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