solana-labs / solana

Web-Scale Blockchain for fast, secure, scalable, decentralized apps and marketplaces.
https://solanalabs.com
Apache License 2.0
13.02k stars 4.19k forks source link

Entangling the runtime with specific loader instructions is inflexible and requires unnecessary overhead #6569

Closed jackcmay closed 4 years ago

jackcmay commented 4 years ago

Problem

This issue encapsulates an extension of the following PR (which has been reverted). https://github.com/solana-labs/solana/pull/6470

Solana runtime requires that all loaders implement LoaderInstruction and wraps the caller's data with InvokeMain. Doing so constrains program writers from extending LoaderInstruction and artificially restricts all non-native programs to using LoaderInstruction

The main goal is to get rid of this: https://github.com/solana-labs/solana/blob/a2a9d54985c140d9b710f2892c95cffe12afd579/runtime/src/message_processor.rs#L144

6470's solution was a partial solution but kept the InvokeMain instruction resulting in the following issues:

Proposed Solution

jackcmay commented 4 years ago

@garious @mvines

garious commented 4 years ago

How would this change the transaction format? Currently, I write Instruction { program_id, data } and that implies InvokeMain of the program with program_id, passing it data. Would InvokeMain need to wrap Instruction::data in the transaction?

jackcmay commented 4 years ago

That was my original proposal but after discussions with @mvines we agreed that removing InvokeMain altogether was a better idea.

garious commented 4 years ago

Have you considered what a general Invoke instruction would look like then? How do I tell the Move VM, "hey, call function "foo" with this data" instead of the main entrypoint?

jackcmay commented 4 years ago

That would be part of ix data sent. The InvokeMain is implicit when there is an executable account. The client/loader are free to decide on the format of the ix data. Maybe the ix data contains something an InvokeMain (like) variant(s), maybe it doesn't

garious commented 4 years ago

Are you saying Move modules shouldn't be marked executable?

garious commented 4 years ago

Yeah, I think that's exactly what you're saying. That there's a difference between an executable Move module that defines main(), and a library that defines a bunch of functions. The latter can be RO, but not executable - that executable is reserved for telling the loader it implemented the known entrypoint and nothing more.

garious commented 4 years ago

So just a thought, but does the runtime have any real dependency on the executable bit? Seems like it only depends on Account::owner. If so, can we make executable the first byte in Account::data for a loader's accounts?

jackcmay commented 4 years ago

(two comments ago) Yes,that would be possible for a loader to do, except in Move's case a library comes bundled in a Move program so main is the entrypoint either way.

jackcmay commented 4 years ago

Runtime does utilize the executable bit both during loading as well as instruction verification. Leaving the Account::data contents up to the loader/programs is probably more flexible

garious commented 4 years ago

Okay, I'm on board. Do it!