VoteProgram.safeWithdraw function to safeguard against accidental vote account closures #26586

aschonfeld · 2022-07-12T14:38:03Z

Problem

Currently when withdrawing rewards from a vote account, the JS SDK can withdraw an amount that will leave the account with less than the minimum required for rent, and will transparently close the account without any confirmation or warning to the developer.

Summary of Changes

To get feature parity with the CLI, we've added a safeWithdraw method to the vote-program. The CLI offers an "ALL" option when withdrawing from a vote account which will leave enough SOL balance to cover the minimum rent for the account. If you accidentally withdraw the entire balance the vote account gets closed.

stale · 2022-08-13T09:01:35Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

joncinque

Sorry for the late review, thanks for your contribution!

web3.js/src/vote-program.ts

aschonfeld · 2022-08-16T14:14:49Z

@joncinque can you take another look when you have some free time please? I've made the adjustments you asked for.

joncinque

We're almost there! This library is a bit of a mess dependency-wise, which is why I don't want to make things worse in this file at least. You show the example in the test -- this way people how know what they're doing will also know how to use it

web3.js/src/programs/vote.ts

web3.js/test/program-tests/vote.test.ts

aschonfeld · 2022-08-16T17:48:56Z

@joncinque one more time please 🙏

joncinque

All right, this is looking good, almost there!

web3.js/src/programs/vote.ts

web3.js/test/program-tests/vote.test.ts

aschonfeld · 2022-08-16T18:51:03Z

@joncinque lets run it back 🤣

codecov · 2022-08-16T19:29:16Z

Codecov Report

Merging #26586 (6b74805) into master (e779032) will decrease coverage by 0.0%.
The diff coverage is n/a.

@@            Coverage Diff            @@
##           master   #26586     +/-   ##
=========================================
- Coverage    76.9%    76.9%   -0.1%     
=========================================
  Files          48       48             
  Lines        2505     2508      +3     
  Branches      355      356      +1     
=========================================
+ Hits         1927     1929      +2     
- Misses        448      450      +2     
+ Partials      130      129      -1

joncinque · 2022-08-16T19:29:50Z

All right, this is good to go on my side, so next we need to pass CI! Could you squash all of your changes down to that first commit to fix the commit lint issue? And then could you check to see the failing test in the vote program test? https://github.com/solana-labs/solana/runs/7865520179?check_suite_focus=true

aschonfeld · 2022-08-16T19:57:37Z

@joncinque ok should be all set now

joncinque · 2022-08-16T20:37:16Z

Sorry, looks like the commit line is too long -- can you shorten it down to 100 characters or less?

aschonfeld · 2022-08-16T21:25:45Z

@joncinque done

joncinque · 2022-08-16T21:33:26Z

I think this commit lint will be too annoying 😓 do you mind if I push to your branch to fix it up and get this merged?

aschonfeld · 2022-08-16T21:36:50Z

Will I lose ownership of the commit?

aschonfeld · 2022-08-16T21:42:07Z

trying one last time...

aschonfeld · 2022-08-16T21:51:26Z

yea you're right this commit-lint thing is awful. If this current one doesn't pass you can push to fix it. I've now shortened it to just feat: safe withdraw function. Pretty cryptic, but oh well.

joncinque · 2022-08-16T22:22:31Z

We made it!

joncinque · 2022-08-16T22:23:05Z

Thanks for your contribution and your patience

…e account closures (solana-labs#26586) feat: safe withdraw function Co-authored-by: aschonfeld <andrew@proofofalpha.io>

* refactor: extract store_stake_accounts fn * refactor: extract store_vote_account fn * refactor: extract reward history update fn * remove avg point value from pay_valiator fn. not used * clippy: slice * clippy: slice * remove abort() from test-validator (#27124) * chore: bump bytes from 1.1.0 to 1.2.1 (#27172) * chore: bump bytes from 1.1.0 to 1.2.1 Bumps [bytes](https://github.com/tokio-rs/bytes) from 1.1.0 to 1.2.1. - [Release notes](https://github.com/tokio-rs/bytes/releases) - [Changelog](https://github.com/tokio-rs/bytes/blob/master/CHANGELOG.md) - [Commits](tokio-rs/bytes@v1.1.0...v1.2.1) --- updated-dependencies: - dependency-name: bytes dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> * [auto-commit] Update all Cargo lock files Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: dependabot-buildkite <dependabot-buildkite@noreply.solana.com> * Share Ancestors API get with contains_key (#27161) consolidate similar fns * Rename to `MAX_BLOCK_ACCOUNTS_DATA_SIZE_DELTA` (#27175) * chore: bump libc from 0.2.129 to 0.2.131 (#27162) * chore: bump libc from 0.2.129 to 0.2.131 Bumps [libc](https://github.com/rust-lang/libc) from 0.2.129 to 0.2.131. - [Release notes](https://github.com/rust-lang/libc/releases) - [Commits](rust-lang/libc@0.2.129...0.2.131) --- updated-dependencies: - dependency-name: libc dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> * [auto-commit] Update all Cargo lock files Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: dependabot-buildkite <dependabot-buildkite@noreply.solana.com> * reverts wide fanout in broadcast when the root node is down (#26359) A change included in #20480 was that when the root node in turbine broadcast tree is down, the leader will broadcast the shred to all nodes in the first layer. The intention was to mitigate the impact of dead nodes on shreds propagation, because if the root node is down, then the entire cluster will miss out the shred. On the other hand, if x% of stake is down, this will cause 200*x% + 1 packets/shreds ratio at the broadcast stage which might contribute to line-rate saturation and packet drop. To avoid this bandwidth saturation issue, this commit reverts that logic and always broadcasts shreds from the leader only to the root node. As before we rely on erasure codes to recover shreds lost due to staked nodes being offline. * add getTokenLargestAccounts rpc method to rust client (#26840) * add get token largest accounts rpc call to client * split to include with commitment * Bump spl-token-2022 (#27181) * Bump token-2022 to 0.4.3 * Allow cargo to bump stuff to v1.11.5 * VoteProgram.safeWithdraw function to safeguard against accidental vote account closures (#26586) feat: safe withdraw function Co-authored-by: aschonfeld <andrew@proofofalpha.io> * chore: bump futures from 0.3.21 to 0.3.23 (#27182) * chore: bump futures from 0.3.21 to 0.3.23 Bumps [futures](https://github.com/rust-lang/futures-rs) from 0.3.21 to 0.3.23. - [Release notes](https://github.com/rust-lang/futures-rs/releases) - [Changelog](https://github.com/rust-lang/futures-rs/blob/master/CHANGELOG.md) - [Commits](rust-lang/futures-rs@0.3.21...0.3.23) --- updated-dependencies: - dependency-name: futures dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> * [auto-commit] Update all Cargo lock files Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: dependabot-buildkite <dependabot-buildkite@noreply.solana.com> * chore: bump nix from 0.24.2 to 0.25.0 (#27179) * chore: bump nix from 0.24.2 to 0.25.0 Bumps [nix](https://github.com/nix-rust/nix) from 0.24.2 to 0.25.0. - [Release notes](https://github.com/nix-rust/nix/releases) - [Changelog](https://github.com/nix-rust/nix/blob/master/CHANGELOG.md) - [Commits](nix-rust/nix@v0.24.2...v0.25.0) --- updated-dependencies: - dependency-name: nix dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> * [auto-commit] Update all Cargo lock files Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: dependabot-buildkite <dependabot-buildkite@noreply.solana.com> * Parse ConfidentialTransaction instructions (#26825) Parse ConfidentialTransfer instructions * snapshots: serialize version file first (#27192) serialize version file first * serialize incremental_snapshot_hash (#26839) * serialize incremental_snapshot_hash * pr feedback * derives Error trait for ClusterInfoError and core::result::Error (#27208) * Add clean_accounts_for_tests() (#27200) * Rust v1.63.0 (#27148) * Upgrade to Rust v1.63.0 * Add nightly_clippy_allows * Resolve some new clippy nightly lints * Increase QUIC packets completion timeout Co-authored-by: Michael Vines <mvines@gmail.com> * docs: updated "transaction fees" page (#26861) * docs: transaction fees, compute units, compute budget * docs: added messages definition * Revert "docs: added messages definition" This reverts commit 3c56156. * docs: added messages definition * Update docs/src/transaction_fees.md Co-authored-by: Jacob Creech <82475023+jacobcreech@users.noreply.github.com> * fix: updates from feedback Co-authored-by: Jacob Creech <82475023+jacobcreech@users.noreply.github.com> * sdk: Fix args after "--" in build-bpf and test-bpf (#27221) * Flaky Unit Test test_rpc_subscriptions (#27214) Increase unit test timeout from 5 seconds to 10 seconds * chore: only buildkite pipelines use sccache in docker-run.sh (#27204) chore: only buildkite ci use sccache * clean feature: `prevent_calling_precompiles_as_programs` (#27100) * clean feature: prevent_calling_precompiles_as_programs * fix tests * fix test * remove comment * fix test * feedback * Add get_account_with_commitment to BenchTpsClient (#27176) * Fix a corner-case panic in get_entries_in_data_block() (#27195) #### Problem get_entries_in_data_block() panics when there's inconsistency between slot_meta and data_shred. However, as we don't lock on reads, reading across multiple column families is not atomic (especially for older slots) and thus does not guarantee consistency as the background cleanup service could purge the slot in the middle. Such panic was reported in #26980 when the validator serves a high load of RPC calls. #### Summary of Changes This PR makes get_entries_in_data_block() panic only when the inconsistency between slot-meta and data-shred happens on a slot older than lowest_cleanup_slot. * Verify snapshot slot deltas (#26666) * store-tool: log lamports for each account (#27168) log lamports for each account * add an assert for a debug feature to avoid wasted time (#27210) * remove redundant call that bumps age to future (#27215) * Use from_secs api to create duration (#27222) use from_secs api to create duration * reorder slot # in debug hash data path (#27217) * create helper fn for clarity (#27216) * Verifying snapshot bank must always specify the snapshot slot (#27234) * Remove `Bank::ensure_no_storage_rewards_pool()` (#26468) * cli: Add subcommands for address lookup tables (#27123) * cli: Add subcommand for creating address lookup tables * cli: Add additional subcommands for address lookup tables * short commands * adds hash domain to ping-pong protocol (#27193) In order to maintain backward compatibility, for now the responding node will hash the token both with and without domain so that the other node will accept the response regardless of its upgrade status. Once the cluster has upgraded to the new code, we will remove the legacy domain = false case. * Revert "Rust v1.63.0 (#27148)" (#27245) This reverts commit a2e7bdf. * correct double negation (#27240) * Enable QUIC client by default. Add arg to disable QUIC client. (Forward port #26927) (#27194) Enable QUIC client by default. Add arg to disable QUIC client. * Enable QUIC client by default. Add arg to disable QUIC client. * Deprecate --disable-quic-servers arg * Add #[ignore] annotation to failing tests * slots_connected: check if the range is connected (>= ending_slot) (#27152) * create-snapshot check if snapshot slot exists (#27153) * Add Bank::clean_accounts_for_tests() (#27209) * Call `AccountsDb::shrink_all_slots()` directly (#27235) * add ed25519_program to built-in instruction cost list (#27199) * add ed25519_program to built-in instruction cost list * Remove unnecessary and stale comment * simple refactorings to disk idx (#27238) * add _inclusive for clarity (#27239) * eliminate unnecessary ZERO_RAW_LAMPORTS_SENTINEL (#27218) * make test code more clear (#27260) * banking stage: actually aggregate tracer packet stats (#27118) * aggregated_tracer_packet_stats_option was alwasys None * Actually accumulate tracer packet stats * Refactor epoch reward 1 (#27253) * refactor: extract store_stake_accounts fn * clippy: slice Co-authored-by: haoran <haoran@mbook> * recovers merkle shreds from erasure codes (#27136) The commit * Identifies Merkle shreds when recovering from erasure codes and dispatches specialized code to reconstruct shreds. * Coding shred headers are added to recovered erasure shards. * Merkle tree is reconstructed for the erasure batch and added to recovered shreds. * The common signature (for the root of Merkle tree) is attached to all recovered shreds. * Simplify `Bank::clean_accounts()` by removing params (#27254) * Account files remove (#26910) * Create a new function cleanup_accounts_paths, a trivial change * Remove account files asynchronously * Update and simplify the implementation after the validator test runs. * Fixes after testing on the dev device * Discard tokio. Use thread instead * Fix comments format * Fix config type to pass the github test * Fix failed tests. Handle the case of non-existing path * Final cleanup, addressing the review comments Avoided OsString. Made the function more generic with "impl AsRef<Path>" Co-authored-by: Jeff Washington <jeff.washington@solana.com> * Refactor: Flattens `TransactionContext::instruction_trace` (#27109) * Flattens TransactionContext::instruction_trace. * Stop the search at transaction level. * Renames get_instruction_context_at => get_instruction_context_at_nesting_level. * Removes TransactionContext::get_instruction_trace(). Adds TransactionContext::get_instruction_trace_length() and TransactionContext::get_instruction_context_at_index(). * Have TransactionContext::instruction_accounts_lamport_sum() accept an iterator instead of a slice. * Removes instruction_trace from ExecutionRecord. * make InstructionContext::new() private * Parallel insertion of dirty store keys during clean (#27058) parallelize dirty store key insertion * Refactor epoch reward 2 (#27257) * refactor: extract store_stake_accounts fn * refactor: extract store_vote_account fn * clippy: slice * clippy: slice * fix merge error Co-authored-by: haoran <haoran@mbook> * Standardize thread names Tenets: 1. Limit thread names to 15 characters 2. Prefix all Solana-controlled threads with "sol" 3. Use Camel case. It's more character dense than Snake or Kebab case * cleanup comment on filter_zero_lamport_clean_for_incremental_snapshots (#27273) * remove inaccurate log (#27255) * patches metrics for invalid cached vote/stake accounts (#27266) patches invalid cached vote/stake accounts metrics Invalid cached vote accounts is overcounting actual mismatches, and invalid cached stake accounts is undercounting. * Refactor epoch reward 3 (#27259) * refactor: extract store_stake_accounts fn * refactor: extract store_vote_account fn * refactor: extract reward history update fn * clippy: slice * clippy: slice Co-authored-by: haoran <haoran@mbook> * fix merges Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: haoran <haoran@mbook> Co-authored-by: Jeff Biseda <jbiseda@gmail.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: dependabot-buildkite <dependabot-buildkite@noreply.solana.com> Co-authored-by: Brooks Prumo <brooks@solana.com> Co-authored-by: behzad nouri <behzadnouri@gmail.com> Co-authored-by: AJ Taylor <anthonjtaylor@gmail.com> Co-authored-by: Tyera Eulberg <tyera@solana.com> Co-authored-by: Andrew Schonfeld <andrew.schonfeld1@gmail.com> Co-authored-by: aschonfeld <andrew@proofofalpha.io> Co-authored-by: apfitzge <apfitzge@users.noreply.github.com> Co-authored-by: Jeff Washington (jwash) <wash678@gmail.com> Co-authored-by: Brennan Watt <brennan.watt@solana.com> Co-authored-by: Michael Vines <mvines@gmail.com> Co-authored-by: Nick Frostbutter <75431177+nickfrosty@users.noreply.github.com> Co-authored-by: Jacob Creech <82475023+jacobcreech@users.noreply.github.com> Co-authored-by: Jon Cinque <jon.cinque@gmail.com> Co-authored-by: Yihau Chen <a122092487@gmail.com> Co-authored-by: Justin Starry <justin@solana.com> Co-authored-by: kirill lykov <kirill.lykov@solana.com> Co-authored-by: Yueh-Hsuan Chiang <93241502+yhchiang-sol@users.noreply.github.com> Co-authored-by: leonardkulms <42893075+leonardkulms@users.noreply.github.com> Co-authored-by: Will Hickey <will.hickey@solana.com> Co-authored-by: Tao Zhu <82401714+taozhu-chicago@users.noreply.github.com> Co-authored-by: Xiang Zhu <xzhu70@gmail.com> Co-authored-by: Jeff Washington <jeff.washington@solana.com> Co-authored-by: Alexander Meißner <AlexanderMeissner@gmx.net>

mergify bot added the community Community contribution label Jul 12, 2022

mergify bot requested a review from a team July 12, 2022 14:38

stale bot added the stale [bot only] Added to stale content; results in auto-close after a week. label Aug 13, 2022

joncinque reviewed Aug 15, 2022

View reviewed changes

web3.js/src/vote-program.ts Outdated Show resolved Hide resolved

stale bot removed the stale [bot only] Added to stale content; results in auto-close after a week. label Aug 15, 2022

aschonfeld force-pushed the safe_withdraw branch from 4985ede to cd4ec09 Compare August 16, 2022 13:53

joncinque reviewed Aug 16, 2022

View reviewed changes

web3.js/src/programs/vote.ts Outdated Show resolved Hide resolved

web3.js/src/programs/vote.ts Outdated Show resolved Hide resolved

web3.js/test/program-tests/vote.test.ts Outdated Show resolved Hide resolved

joncinque reviewed Aug 16, 2022

View reviewed changes

web3.js/src/programs/vote.ts Outdated Show resolved Hide resolved

web3.js/test/program-tests/vote.test.ts Outdated Show resolved Hide resolved

aschonfeld force-pushed the safe_withdraw branch from e150e76 to 8109a19 Compare August 16, 2022 19:57

aschonfeld force-pushed the safe_withdraw branch from 8109a19 to 3844585 Compare August 16, 2022 21:25

aschonfeld force-pushed the safe_withdraw branch from 3844585 to 09a594b Compare August 16, 2022 21:41

feat: safe withdraw function

6b74805

aschonfeld force-pushed the safe_withdraw branch from 09a594b to 6b74805 Compare August 16, 2022 21:50

joncinque merged commit 4b8ab4e into solana-labs:master Aug 16, 2022

steveluscher added this to the web3.js Roadmap – August 2022 milestone Sep 1, 2022

This was referenced Sep 23, 2022

[Snyk] Upgrade @solana/web3.js from 1.31.0 to 1.56.2 Balantion2020/Balantion#468

Open

[Snyk] Upgrade @solana/web3.js from 1.31.0 to 1.58.0 Balantion2020/Balantion#497

Open

[Snyk] Upgrade @solana/web3.js from 1.31.0 to 1.59.1 Balantion2020/Balantion#502

Open

snyk-bot mentioned this pull request Oct 1, 2022

[Snyk] Upgrade @solana/web3.js from 1.31.0 to 1.60.0 Balantion2020/Balantion#515

Open

Digiminey mentioned this pull request Oct 11, 2024

[Snyk] Fix for 2 vulnerabilities Digiminey/solana#201

Open

This was referenced Oct 13, 2024

[Snyk] Fix for 1 vulnerabilities adamlaska/solana#300

Open

[Snyk] Fix for 1 vulnerabilities adamlaska/solana#302

Open

Digiminey mentioned this pull request Oct 18, 2024

[Snyk] Fix for 1 vulnerabilities Digiminey/solana#203

Open

This was referenced Oct 20, 2024

[Snyk] Fix for 1 vulnerabilities adamlaska/solana#308

Open

[Snyk] Fix for 1 vulnerabilities adamlaska/solana#309

Open

[Snyk] Security upgrade @solana/web3.js from 1.50.0 to 1.54.1 adamlaska/solana#312

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

VoteProgram.safeWithdraw function to safeguard against accidental vote account closures #26586

VoteProgram.safeWithdraw function to safeguard against accidental vote account closures #26586

aschonfeld commented Jul 12, 2022

stale bot commented Aug 13, 2022

joncinque left a comment

aschonfeld commented Aug 16, 2022 •

edited

Loading

joncinque left a comment

aschonfeld commented Aug 16, 2022

joncinque left a comment

aschonfeld commented Aug 16, 2022

codecov bot commented Aug 16, 2022 •

edited

Loading

joncinque commented Aug 16, 2022

aschonfeld commented Aug 16, 2022

joncinque commented Aug 16, 2022

aschonfeld commented Aug 16, 2022

joncinque commented Aug 16, 2022

aschonfeld commented Aug 16, 2022

aschonfeld commented Aug 16, 2022

aschonfeld commented Aug 16, 2022

joncinque commented Aug 16, 2022

joncinque commented Aug 16, 2022

VoteProgram.safeWithdraw function to safeguard against accidental vote account closures #26586

VoteProgram.safeWithdraw function to safeguard against accidental vote account closures #26586

Conversation

aschonfeld commented Jul 12, 2022

Problem

Summary of Changes

stale bot commented Aug 13, 2022

joncinque left a comment

Choose a reason for hiding this comment

aschonfeld commented Aug 16, 2022 • edited Loading

joncinque left a comment

Choose a reason for hiding this comment

aschonfeld commented Aug 16, 2022

joncinque left a comment

Choose a reason for hiding this comment

aschonfeld commented Aug 16, 2022

codecov bot commented Aug 16, 2022 • edited Loading

Codecov Report

joncinque commented Aug 16, 2022

aschonfeld commented Aug 16, 2022

joncinque commented Aug 16, 2022

aschonfeld commented Aug 16, 2022

joncinque commented Aug 16, 2022

aschonfeld commented Aug 16, 2022

aschonfeld commented Aug 16, 2022

aschonfeld commented Aug 16, 2022

joncinque commented Aug 16, 2022

joncinque commented Aug 16, 2022

aschonfeld commented Aug 16, 2022 •

edited

Loading

codecov bot commented Aug 16, 2022 •

edited

Loading