replace copy-pasted syscall code with crate import

[kevinheavey]

The contents of syscalls.rs were just copied from Agave and now that code lives in its own crate. There is nothing in solana-define-syscall that pinocchio was not vendoring so this adds no bloat.

Also I had to remove some mentions of "no dependencies" and couldn't think of appropriate song lyrics to replace the existing ones

[sonicfromnewyoke]

personally I like that change, it makes code cleaner IMO.

also, Pinocchio is already dependent on five8, so I don't think adding one more small lib (literally 150 LOC) is going against the philosophy of the project

[febo]

Thanks for the PR - agree that having the code copied is not the best approach. But I wonder if we can discuss whether there is another alternative or not.

Let's take the example of the five8 dependency, which is not a dependency on pinocchio – it is pinocchio-pubkey that has that dependency. This is a good example of the alternative that I would like to propose. So instead of having a dependency tree that "grows" inwards, we add dependency "layers" on top of the main library:

1. "inwards" dependencies

pinocchio
  |
  +- pinocchio-pubkey
       |
       +- five8

2. dependency "layers"

pinocchio-pubkey
  |
  +- five8
  +- pinocchio

One advantage of 2) is that additional dependencies are "opt-in" – you only need pinocchio-pubkey if you want to create a pubkey from a &str.

In this particular case (syscalls), we would remove the code from pinocchio – we might need to keep a small bit of redundancy, but definitely we can reduce the copy-and-paste. Then other crates/code that need "access" to the other syscalls can use the solana-define-syscall directly.

Do you think this (or a variation) could work?

[kevinheavey]

So the syscall wrappers (e.g. pinocchio::pubkey::try_find_program_address, impl_sysvar_get) would move into this new pinocchio-syscalls crate? That would work, I think the only awkward thing to move would be AccountInfo::realloc since it is a method and is much more than a syscall wrapper

[febo]

We could move pinocchio::pubkey::try_find_program_address (and related ones) to pinocchio:pubkey or a new pinocchio:program-derived-address, which then have a dependency on solana-define-syscall. AccountInfo::realloc only uses the sol_memset_ syscall I think. We might be able to replace it by write_bytes or something similar, which the compiler will then use the syscall under the hood.

I think we don't need a pinocchio-syscalls right? Any of the "companion" crates that wrap a syscall can depend directly on solana-definesyscalls`.

[kevinheavey]

And what about the syscall wrappers in log.rs, memory.rs and program.rs?

[febo]

We could move them to different crates:

• memory.rs -> pinocchio-memory (I had a look at solana-program-memory but that one has an extra check on sol_memcpy that would be nice to have the option to bypass + brings the num_traits::SaturatingSub dependency just for that use).
• program.rs -> pinocchio-cpi.

The log.rs one is annoying since msg! is used in "core" places – e.g., panic handler. This is why I mentioned that some redundancy might be needed to make this work. :-) One option is to define the syscall manually on pinocchio (we don't need the macro for that, we just write the single definition directly).

[febo]

@kevinheavey As a side note, we should do the same with the sysvars – make them to be "opt-in", since not all programs use them.