budget as separate contract and system call contract #1189

aeyakovenko · 2018-09-11T21:40:56Z

WIP, part of #953

move budget out of bank
create a system call contract that allocates the account userdata, and assigns account's to contracts

garious · 2018-09-11T21:50:38Z

I see Instruction still depends on Budget. Does it need to?

mvines

I see Instruction still depends on Budget.

Instruction should eventually be an implementation detail of Budget

mvines · 2018-09-11T23:52:56Z

src/transaction.rs

@@ -88,6 +25,7 @@ pub struct Transaction {
    /// In the future which key pays the fee and which keys have signatures would be configurable.
    /// * keys[1] - Typically this is the contract context or the recipient of the tokens
    pub keys: Vec<Pubkey>,
+    pub contract_id: u64,


We don't want this be a PubKey? As u64 we need a central registry

mvines · 2018-09-12T00:05:07Z

src/bank.rs

+                if let Some(id) = contract_id {
+                    accounts[1].contract_id = id;
+                }
+                accounts[1].userdata = vec![0; space as usize];


should we only permit userdata if there's a contract_id?

mvines · 2018-09-12T00:06:59Z

src/bank.rs

    pub fn get_balance(&self, pubkey: &Pubkey) -> i64 {
        self.get_account(pubkey)
-            .map(|x| Self::get_balance_of_budget_payment_plan(&x))
+            .map(|x| budget::get_balance(&x))


we should check that this Account is a budget account first right?

mvines · 2018-09-12T00:09:07Z

src/bank.rs

+#[derive(Serialize, Deserialize, Debug, Clone, Default)]
+pub struct Account {
+    /// tokens in the account
+    pub tokens: i64,


why do we permit negative tokens?

@mvines @garious had the idea that we could have validators vote with a negative fee. so we are supporting the possibility of negative tokens.

Yeah, I still like to believe we'll be able to coalesce transactions from the right to trivialize rollback. I hijacked that top bit to keep the dream alive.

mvines · 2018-09-12T00:12:15Z

src/budget.rs

+/// * accounts[1] - The contract context.  Once the contract has been completed, the tokens can
+/// be spent from this account .
+pub fn process_transaction(tx: &Transaction, accounts: &mut [Account]) -> () {
+    let instruction = deserialize(&tx.userdata).unwrap();


worthwhile assert there are two accounts here? I guess it'll panic in apply_debits_to_budget_state or apply_credits_to_budget_state if not...

aeyakovenko · 2018-09-12T00:50:34Z

@mvines @garious any ideas on naming? budgetstate seems super lame :)

mvines · 2018-09-12T00:59:48Z

BudgetState seems fine to me for now.

mvines · 2018-09-12T00:59:25Z

src/budget.rs

+    last_error: Option<BudgetError>,
+}
+
+pub const BUDGET_STATE_CONTRACT_ID: [u8; 32] = [0u8; 32];


wait, this is the same value as SYSTEM_CALL_CONTRACT_ID. How about:

nothing = 0

SYSTEM_CALL_CONTRACT_ID = 1

BUDGET_STATE_CONTRACT_ID = 2

jackcmay · 2018-09-14T09:36:36Z

src/system.rs

+use transaction::Transaction;
+
+#[derive(Serialize, Deserialize, Debug, Clone)]
+pub enum System {


Should be called SystemContract to align with BudgetContract?

aeyakovenko · 2018-09-15T14:44:35Z

@mvines here is a robust transfer on date that covers trying to pay out to the wrong key
https://github.com/solana-labs/solana/pull/1189/files#diff-ea09cf31a8172005d63e915f6cc56ff7R268

* contract check_id methods * system call contract * verify contract execution rules * move system into its own file * allocate before transfer for budget * store error in budget context * budget contract and tests without bank * moved budget of of bank

mvines · 2018-09-17T17:22:51Z

src/instruction.rs

@@ -0,0 +1,68 @@
+use budget::Budget;


instruction.rs is really just an implementation detail of budget, so (maybe later) this file could merge into budget.rs right?

mvines · 2018-09-17T17:34:34Z

src/bank.rs

@@ -594,7 +555,7 @@ impl Bank {
        {
            let tx = &entry1.transactions[0];
            let instruction = tx.instruction();
-            let deposit = if let Instruction::NewContract(contract) = instruction {
+            let deposit = if let Some(Instruction::NewContract(contract)) = instruction {


Couldn't we pass tx through process_transaction() here instead of hard coding a Budget Contract instruction?

@mvines maybe, not sure. its outside the scope of moving budget as a separate contract

aeyakovenko · 2018-09-17T18:56:26Z

needs a wool update

mvines · 2018-09-17T20:08:10Z

This looks good enough to land IMO,

…na-labs#1189) Bumps [rollup](https://github.com/rollup/rollup) from 2.38.4 to 2.38.5. - [Release notes](https://github.com/rollup/rollup/releases) - [Changelog](https://github.com/rollup/rollup/blob/master/CHANGELOG.md) - [Commits](rollup/rollup@v2.38.4...v2.38.5) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

…#1189)

…port of #1189) (#1208) RPC: fix numNonVoteTransactions in getPerformanceSamples (#1189) (cherry picked from commit 3f6c567) Co-authored-by: Justin Starry <justin@anza.xyz>

…port of solana-labs#1189) (solana-labs#1208) RPC: fix numNonVoteTransactions in getPerformanceSamples (solana-labs#1189) (cherry picked from commit 3f6c567) Co-authored-by: Justin Starry <justin@anza.xyz>

aeyakovenko added work in progress This isn't quite right yet noCI Suppress CI on this Pull Request labels Sep 11, 2018

aeyakovenko requested a review from mvines September 11, 2018 21:40

mvines reviewed Sep 12, 2018

View reviewed changes

mvines mentioned this pull request Sep 12, 2018

RFC 006 - Budget Contract Language #1193

Closed

CriesofCarrots mentioned this pull request Sep 13, 2018

Wallet CLI: add handling to separately compile and transfer raw userdata #1203

Closed

mvines mentioned this pull request Sep 14, 2018

Pubkey duplication in Transaction #1210

Closed

jackcmay reviewed Sep 14, 2018

View reviewed changes

aeyakovenko mentioned this pull request Sep 15, 2018

Example of how Budget::After is broken #1221

Closed

aeyakovenko force-pushed the budget_contract branch from 3d7d0f9 to f07f685 Compare September 17, 2018 14:58

aeyakovenko removed noCI Suppress CI on this Pull Request work in progress This isn't quite right yet labels Sep 17, 2018

aeyakovenko requested review from garious, mvines and jackcmay September 17, 2018 14:59

aeyakovenko added 2 commits September 17, 2018 08:01

comment

1f4ea9b

clippy

60d7188

mvines reviewed Sep 17, 2018

View reviewed changes

mvines added this to the v0.9 Swamis milestone Sep 17, 2018

comments

a6c45cb

aeyakovenko merged commit 6ec0e42 into solana-labs:master Sep 17, 2018

aeyakovenko mentioned this pull request Sep 17, 2018

Smart Contracts Engine Backend #953

Closed

6 tasks

steviez pushed a commit to steviez/solana that referenced this pull request May 7, 2024

RPC: fix numNonVoteTransactions in getPerformanceSamples (solana-labs…

3f6c567

…#1189)

CriesofCarrots pushed a commit to CriesofCarrots/solana that referenced this pull request May 7, 2024

RPC: fix numNonVoteTransactions in getPerformanceSamples (solana-labs…

437b3ce

…#1189)

budget as separate contract and system call contract #1189

budget as separate contract and system call contract #1189

Uh oh!

Conversation

aeyakovenko commented Sep 11, 2018

Uh oh!

garious commented Sep 11, 2018

Uh oh!

mvines left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aeyakovenko commented Sep 12, 2018

Uh oh!

mvines commented Sep 12, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aeyakovenko commented Sep 15, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aeyakovenko commented Sep 17, 2018

Uh oh!

mvines commented Sep 17, 2018

Uh oh!

Uh oh!