-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Reject blocks for costs above the max block cost #18994
Merged
tao-stones
merged 7 commits into
solana-labs:master
from
tao-stones:reject_too_large_blocks
Aug 12, 2021
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
8448223
added realtime cost checking logic to reject block that would exceed …
tao-stones 888ef92
update abi that changed due to adding additional TransactionError
tao-stones 210eb4e
To avoid counting stats mltiple times, only accumulate execute-timing…
tao-stones b87effd
gate it by a feature
tao-stones 9de8fa5
move cost const def into block_cost_limits.rs
tao-stones 5456e2a
redefine the cost for signature and account access, removed signer pa…
tao-stones af1befb
check if per_program_timings of execute_timings before sending
tao-stones File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
//! defines block cost related limits | ||
//! | ||
|
||
// see https://github.com/solana-labs/solana/issues/18944 | ||
// and https://github.com/solana-labs/solana/pull/18994#issuecomment-896128992 | ||
// | ||
pub const MAX_BLOCK_TIME_US: u64 = 400_000; // aiming at 400ms/block max time | ||
pub const AVG_INSTRUCTION_TIME_US: u64 = 1_000; // average instruction execution time | ||
pub const SYSTEM_PARALLELISM: u64 = 10; | ||
pub const MAX_INSTRUCTION_COST: u64 = 200_000; | ||
pub const MAX_NUMBER_BPF_INSTRUCTIONS_PER_ACCOUNT: u64 = 200; | ||
|
||
pub const fn max_instructions_per_block() -> u64 { | ||
(MAX_BLOCK_TIME_US / AVG_INSTRUCTION_TIME_US) * SYSTEM_PARALLELISM | ||
} | ||
|
||
pub const fn block_cost_max() -> u64 { | ||
MAX_INSTRUCTION_COST * max_instructions_per_block() | ||
} | ||
|
||
pub const fn account_cost_max() -> u64 { | ||
MAX_INSTRUCTION_COST * max_instructions_per_block() | ||
} | ||
|
||
pub const fn compute_unit_to_us_ratio() -> u64 { | ||
block_cost_max() / MAX_BLOCK_TIME_US | ||
} | ||
|
||
pub const fn signature_cost() -> u64 { | ||
// signature takes average 10us | ||
compute_unit_to_us_ratio() * 10 | ||
} | ||
|
||
pub const fn account_read_cost() -> u64 { | ||
// read account averages 5us | ||
compute_unit_to_us_ratio() * 5 | ||
} | ||
|
||
pub const fn account_write_cost() -> u64 { | ||
// write account averages 25us | ||
compute_unit_to_us_ratio() * 25 | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
So if
!bank.is_complete()
, then it just sends completely emptyexecute_timings
?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.
yea, it'd send empty
execute_timings
if no bank is complete in this loop. However, on the receiving side, empty timings are ignored (https://github.com/solana-labs/solana/blob/master/core/src/cost_update_service.rs#L140-L143).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 waste the cycles of putting the thing in the channel and receiving if it just ignores it though? Hard to follow that kind of logic too, where it does extra work and nothing happens.
Can we not just send at the end of the block?
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.
yea, fair enough. Originally thought was to keep the flow uniform, at cost of wasted cycles. Can add a small check at the end of function before sending