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

Fix small problems in transaction-dos tool #23234

Merged
merged 3 commits into from
Feb 23, 2022

Conversation

KirillLykov
Copy link
Contributor

Problem

Test (ignored) for transaction-dos tool didn't work.

Summary of Changes

  1. specify path to the program with CARGO_MANIFEST_BUILD to prevent problems when running from different directories
  2. replaced transaction size upper bound with PACKET_DATA_SIZE constant
  3. use correct blockhash not only for transaction but also for message to get fee value

transaction-dos/src/main.rs Outdated Show resolved Hide resolved
transaction-dos/src/main.rs Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Feb 18, 2022

Codecov Report

Merging #23234 (bb1c3de) into master (c696944) will decrease coverage by 0.0%.
The diff coverage is n/a.

❗ Current head bb1c3de differs from pull request most recent head b73f95d. Consider uploading reports for the commit b73f95d to get more accurate results

@@            Coverage Diff            @@
##           master   #23234     +/-   ##
=========================================
- Coverage    81.3%    81.3%   -0.1%     
=========================================
  Files         570      570             
  Lines      155438   155438             
=========================================
- Hits       126452   126450      -2     
- Misses      28986    28988      +2     

tao-stones
tao-stones previously approved these changes Feb 18, 2022
Copy link
Contributor

@tao-stones tao-stones left a comment

Choose a reason for hiding this comment

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

lgtm except that nit.

@mergify mergify bot dismissed tao-stones’s stale review February 21, 2022 21:06

Pull request has been modified.

@ryleung-solana
Copy link
Contributor

Lgtm

ryleung-solana
ryleung-solana previously approved these changes Feb 21, 2022
@mergify mergify bot dismissed ryleung-solana’s stale review February 21, 2022 21:13

Pull request has been modified.

@KirillLykov KirillLykov merged commit 05f04a2 into solana-labs:master Feb 23, 2022
jeffwashington pushed a commit to jeffwashington/solana that referenced this pull request Mar 4, 2022
* Fix small problems in transaction-dos tool
@KirillLykov KirillLykov deleted the fix_transaction_dos branch March 23, 2022 09:18
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.

3 participants