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

Make ELF callable via pubkey #1378

Closed
garious opened this issue Sep 27, 2018 · 23 comments
Closed

Make ELF callable via pubkey #1378

garious opened this issue Sep 27, 2018 · 23 comments
Assignees
Milestone

Comments

@garious
Copy link
Contributor

garious commented Sep 27, 2018

Add a way to upload an interpreter (large compiled userdata) to a single account so that it can be used as a program_id in subsequent transactions.

@garious garious added this to the v0.10 Pillbox milestone Sep 27, 2018
@garious
Copy link
Contributor Author

garious commented Oct 2, 2018

Depends on #940

@garious
Copy link
Contributor Author

garious commented Oct 8, 2018

@CriesofCarrots, @mvines, I unassigned @jackcmay here. One of you can implement this without stepping into the code he's working on. Just extend SystemProgram with a way to target a particular offset within the userdata.

@mvines
Copy link
Member

mvines commented Oct 8, 2018

Hmm, I don't think we want to extend the SystemProgram in this way. The ELF should have exactly one entrypoint

@garious
Copy link
Contributor Author

garious commented Oct 8, 2018

It'd still have just one entry point. This is just about getting the bits of a large userdata into a single account.

@mvines
Copy link
Member

mvines commented Oct 8, 2018

Ah, yes. This is part of the "load" instruction Jack and I have talked about. Was planning to see if @CriesofCarrots could take this on after her current pubsub work

@jackcmay
Copy link
Contributor

jackcmay commented Oct 9, 2018

I see this is a generic operation (could apply to storage, programs, whatever) that just loads an account with data over multiple transactions.

@mvines
Copy link
Member

mvines commented Oct 9, 2018

I'm not convinced we should open this up as a general purpose facility to populate account userdata. I'm worried this opens up an attack vector for programs:

  1. Load up a system account with attack data
  2. Assign the attack account to the victim program
  3. Transact with the victim program using the attack account.
  4. Profit

Right now programs can assume that either their accounts will be all zeros, or that only they have set some of those zeros to ones.

@garious
Copy link
Contributor Author

garious commented Oct 9, 2018

How is that different than what you can do with the API as-is?

@mvines
Copy link
Member

mvines commented Oct 9, 2018

Only programs can write into account userdata right now, and as a user you can only give a program a zero initialized account.

The difference between sending data to a program on standard input versus writing into the program's heap.

@jackcmay
Copy link
Contributor

jackcmay commented Oct 9, 2018

@greg I'd like to stay assigned to this ticket since I'm already a lot of the way through the bulk of it. What someone could work on on the side is a way to load a large amount of data into an account. This would require breaking up the data into packets and reassembling them into the account. Could be as simple as providing an offset and data chunk for each packet.

@mvines I see your point about only allowing the program to modify its state as you don't want someone priming the program with a state that wasn't derived via a series of signed program inputs. This implies that the one who starts the program does not have any more control over the program than anyone else.

@mvines
Copy link
Member

mvines commented Oct 9, 2018

What someone could work on on the side is a way to load a large amount of data into an account. This would require breaking up the data into packets and reassembling them into the account. Could be as simple as providing an offset and data chunk for each packet.

Yeah this (aka, #1378 (comment)) could be a separate, parallel effort.

This implies that the one who starts the program does not have any more control over the program than anyone else.

Yep for sure. By default we want that to be true. The program itself can be more liberal itself if it so chooses.

@garious
Copy link
Contributor Author

garious commented Oct 9, 2018

I get the feeling we're of mixed opinions of what this ticket is tracking. @jackcmay, can you update to it to best describe what you're implementing. @mvines, is there another ticket already tracking this "can't load account with large data" issue? If no, can you create one and assign that to @CriesofCarrots?

@jackcmay
Copy link
Contributor

jackcmay commented Oct 9, 2018

This ticket covers exporting an interface that allows for calling a program by providing the pubkey of the program one wishes to call and the state they wish to call it with.

    fn program_call(
        from_keypari: &Keypair,
        accounts: &Vec<Pubkey>,
        input: Vec<u8>,
        last_id: Hash,
        fee: i64,
    ) -> Self;

Where accounts contain the program pubkey, state pubkey, and any other account that the contract wants to access.

@jackcmay
Copy link
Contributor

jackcmay commented Oct 9, 2018

#1454 Opened and assigned to @CriesofCarrots. @CriesofCarrots let me know before you start work on this as I might be in a position to take it over.

@garious
Copy link
Contributor Author

garious commented Oct 9, 2018

That looks good to me, @jackcmay. Just accounts can be named keys doesn't need the &. Also, all constructors should start with new_.

@jackcmay
Copy link
Contributor

jackcmay commented Oct 9, 2018

How about this (name change and program and state keys are more explicit)?

    fn program_new_call(
        from_keypari: &Keypair,
        program: Pubkey,
        state: Pubkey,
        accounts: &[Pubkey],
        input: Vec<u8>,
        last_id: Hash,
        fee: i64,
    ) -> Self;

@mvines
Copy link
Member

mvines commented Oct 9, 2018

My vote:

fn program_new_call(
        from_keypair: &Keypair,
        program_id: Pubkey,
        accounts: &[Pubkey],
        userdata: Vec<u8>,
        last_id: Hash,
        fee: i64,
    ) -> Self;

We use program_id instead of program everywhere else. account[0] may be state by convention but we don't need to bake that assumption in.

@garious
Copy link
Contributor Author

garious commented Oct 9, 2018

Any reason why we don't toss this into SystemProgram?

Also, it's been standard practice to use move semantics when the data it just going to be dropped into a struct. Saves a bunch of cloning:

fn system_new_call(
        from_keypair: &Keypair,
        program_id: Pubkey,
        keys: Vec<Pubkey>,
        userdata: Vec<u8>,
        last_id: Hash,
        fee: i64,
    ) -> Self;

@garious
Copy link
Contributor Author

garious commented Oct 9, 2018

@mvines
Copy link
Member

mvines commented Oct 9, 2018

Transaction::new() looks great

@jackcmay
Copy link
Contributor

jackcmay commented Oct 9, 2018 via email

@jackcmay
Copy link
Contributor

@garious Anything left to do for this issue?

@mvines
Copy link
Member

mvines commented Oct 25, 2018

Yeah this appears to be done

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

No branches or pull requests

3 participants