MT safe bank and atomic multiple program execution #1309

aeyakovenko · 2018-09-22T22:50:02Z

Make bank transaction processing MT safe. This also fixed a bug where a multiple spend within a batch was generating extra tokens :)
Allow for transactions to send multiple programs that will execute as one atomic operation
The above required a change to the transaction encoding and the ABI between the bank and the programs. (@garious there is an unsafe call to create a subset of mutable references that I couldn't find a way around https://github.com/solana-labs/solana/pull/1309/files#diff-7145777648f8c12caeb15b4e1755215cR438)
Store error codes for signatures

@garious @mvines before i rebase, let me know if this covers all the things we talked about last week. i know its a lot of changes but it all came together really well. Take a look at the new bank tests (https://github.com/solana-labs/solana/pull/1309/files#diff-7145777648f8c12caeb15b4e1755215cR872), that should showcase the improvements. Also, how do i update the sdk golden image tests? A big TODO is to pass multiple signatures in the TX, ill need some help from @sakridge with changing the cuda to support that

So main reason why it all came together in one change was that to fix the bug and still allow for context free contract execution the accounts need to be "locked" while they are being changed in the bank's pipeline. which means that spending from 1 to many would be bottlenecked by a single transaction thats in the pipeline.

The batching implementation is most of #1293, the main differences are

keep the signed data in the transaction data
use a program_keys vector in the Transaction
use old names to reduce churn in other parts of the codebase

1 and 3 can be addressed later. For 2, this allows us to pack more things into one transaction. The reason why they are not stored as "keys" is because the engine doesn't distinguish RO from RW accounts in the transaction.

Bench results (mac)

bench_banking_stage_multi_programs ... bench: 28,720,294 ns/iter (+/- 9,772,506)
bench_banking_stage_multi_accounts ... bench: 13,838,541 ns/iter (+/- 8,367,130)

Multi programs process about 8*1024*5 programs per iteration (5 per tx that are spent atomically). 700ns per program.
Multi accounts processes 8*1024 transactions. 1.6us per atomic transaction

@rob-solana
Poh service is the entry generator and also what drives last_id registration into the bank.
https://github.com/solana-labs/solana/pull/1309/files#diff-a731213f65ed3e3b9e6129eb03dc9efeR121
There is a race between the entry output, and last_id's in the bank. The last_id queue has to move in lockstep with whats in the "ledger", which is the Entry output. With multiple threads driving the bank the output and updates to LastId queue need to be linearized.

garious · 2018-09-24T14:43:53Z

I was surprised this PR doesn't reference #1293 and explain why it deviates from that design. Overloading of the term Program looks especially problematic.

For the golden image tests, there's a bunch of assert_eq!(actual, expected). When you change the binary format, those tests will fail. To fix them, copy the output from "left" (aka actual) back into the test.

aeyakovenko · 2018-09-24T15:41:07Z

@garious left comment about #1293

garious · 2018-09-24T15:53:26Z

Using old names to reduce churn makes this PR unnecessarily difficult to grok. Can you submit a separate PR that just changes the names to free up the namespace for this PR? instruction.rs -> budget_instruction.rs looks like the big one.

aeyakovenko · 2018-09-24T17:52:42Z

@garious @mvines did we decide on names? #1293 has some of it documented, but its not the main point of that PR.

sakridge · 2018-09-24T18:37:52Z

benches/banking_stage.rs

-
+    transactions.iter().for_each(|tx| {
+        let fund = Transaction::new(
+            &mint.keypair(),


This always transfers from the mint right? This is meant to transfer from multiple source accounts.

@sakridge yea, thats why the initial funding is synchronous

sakridge · 2018-09-24T18:39:04Z

src/banking_stage.rs

 use std::thread::{self, Builder, JoinHandle};
 use std::time::Duration;
 use std::time::Instant;
 use timing;
 use transaction::Transaction;

+pub const NUM_THREADS: usize = 8;


fixed number of threads?

yea, for now. we can make this dynamic later

Implement an mt safe bank and batch programs into single atomic transactions. fix entry processing fmt once! cleanup and remove clone recv poh until response arrives handle recverrors fix benches get rid of answer, use 10 threads because math is simpler coalesce the extra thread switch optimizing cleanup register faster cleanup fixed test sdk serialize fixed benches rebase program counter fmt

rob-solana · 2018-09-26T18:47:57Z

I'd move tick_producer thread into poh_service, then we'll have come full circle back to record_stage

rob-solana · 2018-09-26T18:53:08Z

src/poh_service.rs

        let mut poh = self.poh.lock().unwrap();
-        poh.record(mixin)


drop this lock after record/tick()?

rob-solana · 2018-09-26T19:24:03Z

I've been eyeballing this for a while and it looks like it'll work WRT lastidnotfound

garious · 2018-09-26T20:28:49Z

src/transaction.rs

@@ -15,6 +15,17 @@ pub const SIGNED_DATA_OFFSET: usize = size_of::<Signature>();
 pub const SIG_OFFSET: usize = 0;
 pub const PUB_KEY_OFFSET: usize = size_of::<Signature>() + size_of::<u64>();

+#[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone)]
+pub struct Program {


If you rebase, you can rename this to Instruction now. The term Program is awkward here because that program_id field is not a unique identifier for instances of this struct, and because the stuff serialized in the userdata may be a new program or a call into an existing one. Maybe this should be an enum too?

aeyakovenko · 2018-09-27T18:09:54Z

closing because most of this has been broken out into smaller PRs. Thank you for the feedback!

…-labs#1309) Bumps [@babel/plugin-proposal-class-properties](https://github.com/babel/babel/tree/HEAD/packages/babel-plugin-proposal-class-properties) from 7.12.13 to 7.13.0. - [Release notes](https://github.com/babel/babel/releases) - [Changelog](https://github.com/babel/babel/blob/main/CHANGELOG.md) - [Commits](https://github.com/babel/babel/commits/v7.13.0/packages/babel-plugin-proposal-class-properties) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

…s#1309) * bump stable to v1.78.0 * bump nightly to 2024-05-02 * clippy: non_local_definitions

aeyakovenko requested review from garious, mvines, CriesofCarrots and jackcmay September 22, 2018 22:50

sakridge reviewed Sep 24, 2018

View reviewed changes

aeyakovenko force-pushed the mt_bank branch from a03ad32 to cf4b34c Compare September 24, 2018 19:23

aeyakovenko requested a review from rob-solana September 25, 2018 00:10

aeyakovenko force-pushed the mt_bank branch 2 times, most recently from 131c737 to 19a1a9e Compare September 25, 2018 21:07

aeyakovenko added 2 commits September 26, 2018 09:38

builds after rebase

07ddca8

aeyakovenko force-pushed the mt_bank branch from fc87682 to 3be9e66 Compare September 26, 2018 16:39

rob-solana reviewed Sep 26, 2018

View reviewed changes

src/poh_service.rs

let mut poh = self.poh.lock().unwrap();

poh.record(mixin)

Copy link

Contributor

rob-solana Sep 26, 2018

Choose a reason for hiding this comment

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

drop this lock after record/tick()?

garious reviewed Sep 26, 2018

View reviewed changes

aeyakovenko closed this Sep 27, 2018

garious mentioned this pull request Oct 11, 2018

Permit batching transactions #1293

Closed

ryoqun pushed a commit to ryoqun/solana that referenced this pull request May 29, 2024

chore: bump rust stable to v1.78.0, nightly to 2024-05-02 (solana-lab…

1f3ef33

…s#1309) * bump stable to v1.78.0 * bump nightly to 2024-05-02 * clippy: non_local_definitions

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MT safe bank and atomic multiple program execution #1309

MT safe bank and atomic multiple program execution #1309

aeyakovenko commented Sep 22, 2018 •

edited

Loading

garious commented Sep 24, 2018

aeyakovenko commented Sep 24, 2018

garious commented Sep 24, 2018

aeyakovenko commented Sep 24, 2018

sakridge Sep 24, 2018

aeyakovenko Sep 24, 2018

sakridge Sep 24, 2018

aeyakovenko Sep 25, 2018

rob-solana commented Sep 26, 2018

rob-solana Sep 26, 2018

rob-solana commented Sep 26, 2018

garious Sep 26, 2018

aeyakovenko commented Sep 27, 2018

MT safe bank and atomic multiple program execution #1309

MT safe bank and atomic multiple program execution #1309

Conversation

aeyakovenko commented Sep 22, 2018 • edited Loading

garious commented Sep 24, 2018

aeyakovenko commented Sep 24, 2018

garious commented Sep 24, 2018

aeyakovenko commented Sep 24, 2018

sakridge Sep 24, 2018

Choose a reason for hiding this comment

aeyakovenko Sep 24, 2018

Choose a reason for hiding this comment

sakridge Sep 24, 2018

Choose a reason for hiding this comment

aeyakovenko Sep 25, 2018

Choose a reason for hiding this comment

rob-solana commented Sep 26, 2018

rob-solana Sep 26, 2018

Choose a reason for hiding this comment

rob-solana commented Sep 26, 2018

garious Sep 26, 2018

Choose a reason for hiding this comment

aeyakovenko commented Sep 27, 2018

aeyakovenko commented Sep 22, 2018 •

edited

Loading