token-2022: implement CpiGuard

[2501babe]

closes #3694

[2501babe]

well the first pass on the processor and instruction is basically done. i copied everything from RequiredTransferMemos almost exactly, partly because it does exactly what i need, and partly because im unsure about how closely i need to adhere to a house style for mainnet programs

[2501babe]

alright the hardest part of this entire thing honestly was setting up the ability to test calls to token22 through a cpi intermediary and that is dooone. and i have correct tests for the "cpi guard cant be changed in cpi" bullet point. everything works. next up: add the guards into the token instructions!

[2501babe]

with fresh eyes on this after the weekend, im not sure about my cpi testing strategy. i wrote this like, each testable action gets one passthrough program instruction that performs the required action, obviating the need to use the data array for anything or implement serialization logic. but now im wondering if it wouldnt be better if the passthrough took an arbitrary serialized instruction, parsed, and invoked it, obviating the need for multiple instructions on the passthrough?

im gonna keep going like this since i already set the infra up to work this way (and also because the other way is more complicated lol) but maybe something to consider after the feature is done...

[2501babe]

@joncinque the extension is done, if you wanted to take a peek. the only interesting files are program-2022/src/processor.rs (the guards themselves) and program-2022/src/error.rs (detailed explanations of what they all mean). the extension itself is largely copy-pasted from required memo transfers, and all the other files are test infra, most of which ill be summarily deleting in favor of your instruction wrapper

also ty for telling me about get_stack_height()! it made coding this shockingly simple compared to what i thought id have to do

[joncinque]

The logic looks great! Just a few little questions and nits

[joncinque]

nit: the native mode of program-test, ie the mode set using program_test.prefer_bpf(false), should actually work rather than needing to build this in BPF and use it in program-2022-test. That way, you don't need to worry about building the program separately, and instead just include its processor function

[2501babe]

im assuming this is meant to be a comment on my adding my caller program to build.rs, which i assumed was necessary (and added your instruction padder to it to replace.) will try removing it and see if things still work but if you meant something else lmk

edit: well. it turns out build.rs doesnt do anything for me in either case. i added a panic to the processor and tests kept passing anyway

it looks like the way things are currently set up, the tests just use whatever version of the instruction padder sbf binary has already been built by hand; my version of build.rs doesnt force a rebuild if the padder changes, and the tests are definitely not using the processor function as-is (otherwise my tests would be failing in ci because of the get_stack_height() issue)

im gonna revert my build.rs changes since it doesnt seem to affect anything either way. i cargo culted this one based on what was already there without any awareness of what it actually is supposed to do

[2501babe]

lol jk i guess without it in build.rs, ci doesnt build it at all

[2501babe]

im now based on the instruction padder and using it for tests. ill address the rest of the fixmes and write tests for the remaining fns tomorrow. also since im removing the caller thing iwrote the commit history is fucked so i will be squash merging this (ive become very fond of rebase merging...)

[2501babe]

@joncinque just to clarify when you have a chance:

• was verdict on close account to leave it as it is, or further restrict to native mint?
• would you be open to a pr to prevent setting close authority on an account with ImmutableOwner?

[joncinque]

was verdict on close account to leave it as it is, or further restrict to native mint?

Sorry, I was unclear on this -- I think it's good to leave it as it is

would you be open to a pr to prevent setting close authority on an account with ImmutableOwner?

I'm probably missing something here, since I don't totally understand the rationale. If an account has an immutable owner, an outside close authority doesn't threaten or change the immutable ownership of the account. You still can't set a different owner. Or is the concept that the close authority exerts some form of "ownership" over the account?

[2501babe]

yea i guess something like "granting a close authority is tantamount to partial alienation of ownership" but if that doesnt make sense im not like, strongly in favor

[joncinque]

In that case, my vote is to leave it out for now

[2501babe]

alright tests are done, final fixes tomorrow and then we should be good

[2501babe]

ok i think this is done. note again i will be squashing this hstory lol

[joncinque]

great work! only micro things to do with as you please

Pull request has been modified.

[t-nelson]

looks like this broke the monorepo ci downstream-builds job somehow. it's complaining about the shared object not being available, despite a shiny green "compiling" above and no suspicious log lines in between. 🤷

gonna revert in the meantime. feel free to put it back once resolved

EDIT:
@yihau found a workaround, which i think is correct on its own since we should be testing that program upstream as well, but i think these tests should be self-contained wrt dependency resolution as well, right?