feat(lang-v2): support Sysvar<SysvarInstructions> for introspection - #4944
feat(lang-v2): support Sysvar<SysvarInstructions> for introspection#4944franRappazzini wants to merge 4 commits into
Sysvar<SysvarInstructions> for introspection#4944Conversation
`Sysvar<T>` only wrapped syscall-backed sysvars, so instruction introspection was unreachable from `#[derive(Accounts)]`. Split the wrapper's bound in two: `SysvarId` still supplies the well-known address, and a new `SysvarLoad` says how to read the value. `Clock` / `Rent` read from `sol_get_sysvar`; `Instructions` has no syscall and borrows the account data instead, holding a `'static` `Ref` guard for the wrapper's lifetime as `SerializedAccount::load` does. No proc-macro change needed — v2 dispatches account types by trait. Covered by wrapper unit tests over a synthetic sysvar blob, a Miri witness for the transmute, and LiteSVM tests on real SBF
|
@franRappazzini is attempting to deploy a commit to the OtterSec Team on Vercel. A member of the Team first needs to authorize it. |
swaroop-osec
left a comment
There was a problem hiding this comment.
lgtm!
Only concern is about Instructions import in Prelude which can cause name collisions. Since it's a generic name programs commonly use for their own types
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## anchor-next #4944 +/- ##
==============================================
Coverage ? 47.90%
==============================================
Files ? 158
Lines ? 31180
Branches ? 0
==============================================
Hits ? 14938
Misses ? 16242
Partials ? 0
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
#4944 looks like the right PR to take, but before we merge could you please rename the public alias to SysvarInstructions instead of exporting bare Instructions from the prelude? The bare name creates a real downstream prelude collision for crates that already define their own Instructions type and then write |
… note - Renamed the `Instructions` type alias to `SysvarInstructions` to prevent namespace collisions in the prelude for downstream programs. - Removed the hardcoded list of supported sysvars from the `SysvarLoad` trait's diagnostic note to reduce maintenance overhead.
Sysvar<Instructions> for introspectionSysvar<SysvarInstructions> for introspection
|
@jamie-osec @swaroop-osec @0x4ka5h already checked and ready for merge! I updated the name for |
Summary
Sysvar<T>was bound toT: PinocchioSysvar + SysvarId + Copy, whichonly admits sysvars the runtime exposes through
sol_get_sysvar. Theinstructions sysvar is not one of them, so instruction introspection —
reading the other instructions in the current transaction — was
unreachable from a
#[derive(Accounts)]struct.This PR makes
Sysvar<SysvarInstructions>a first-class account type:Design
Splitting
SysvarIdfromSysvarLoadThe two sysvar families are read in fundamentally different ways, so the
single trait that drove the wrapper is split in two:
SysvarId(existing)SysvarLoad(new)Clock/Rentread through the syscall and never touch account data.Instructionshas no syscall at all — its value must be read out of theaccount's data buffer, which means the account has to be passed in the
transaction and the wrapper has to keep a borrow of it.
Sysvar<T>'s bound relaxes to justSysvarLoad(which requiresSysvarId), and the now-redundantPhantomData<T>is dropped since thevalue is already stored by value.
Why syscall sysvars get a macro instead of a blanket impl
A blanket
impl<T: PinocchioSysvar> SysvarLoad for Twould overlap theSysvarInstructionsimpl, and rustc cannot rule the overlap out: provingSysvarInstructions: !PinocchioSysvaris negative reasoning about a foreigntrait on a foreign type. Each syscall-backed sysvar therefore gets an
explicit impl, generated by a small
impl_syscall_sysvar!macro thatpairs the address, the IDL string, and the
get()call in one place.The
Instructionsalias and its'staticborrowpinocchio's
Instructions<T>is generic over its data container. Thenew alias pins it at the one
Tthat can outliveload:SysvarLoad::readborrows the account data and transmutes the guard to'static. This is the same patternSerializedAccount::loadalreadyuses:
Refstores raw pointers into runtime memory rather than into theAccountView, so moving the view into the wrapper afterwards does notinvalidate the guard. Holding it for the wrapper's lifetime is what
blocks later mutable borrows of the same account.
Sysvar::loadhas already compared the address againstINSTRUCTIONS_IDby the time
readruns, so it goes throughnew_uncheckedrather thanpinocchio's
TryFrom<&AccountView>— that avoids redoing the compare andlets the
Refbe transmuted alone instead of the wholeInstructions<_>.Under the
guardrailsfeature,readalso rejects a buffer too small tohold the
[u16 num][u16 current_index]skeleton, so a hand-rolled mockview cannot underflow the pointer arithmetic in
load_current_index.The address check makes this unreachable for the genuine sysvar.
No proc-macro change
v2 resolves account types through trait dispatch rather than a name
table, so
#[derive(Accounts)]picks the wrapper up with no codegenchange. The
SysvarIdimpl on the genericInstructions<T>is kept sothe IDL address is available for any instantiation; only the alias — the
instantiation that can outlive
load— getsSysvarLoad.Tests
Wrapper unit tests (
lang-v2/tests/account_wrapper_checks.rs) — fivecases driving a synthetic sysvar blob built to the runtime's exact
layout, so the pointer arithmetic in pinocchio's accessors is exercised
for real rather than mocked:
sysvar_instructions_reads_synthetic_blobsysvar_instructions_load_rejects_wrong_addresssysvar_instructions_rejects_undersized_datasysvar_instructions_rejects_out_of_range_indexsysvar_instructions_holds_a_shared_borrow_not_an_exclusive_oneMiri witness (
lang-v2/tests/miri_wrapper_accounts.rs) — runs underTree Borrows in CI. Pins the claim behind the
unsafe: the guard'sprovenance survives the view being moved into the wrapper, and the borrow
flag is released exactly once on drop.
Integration on real SBF (
tests-v2/) — aread_instructionshandlerthat introspects its own invocation through LiteSVM, asserting from
inside the program that relative index 0 carries the right program id,
this handler's discriminant, and the sysvar as its only readonly account:
read_instructions_introspects_the_current_instructionread_instructions_rejects_wrong_sysvar— passingRentinstead tripsthe address compare before any data is borrowed
Diagnostics — the
SlotHashescompile-fail case now asserts onSysvarLoad, the bound an unsupported sysvar actually trips, and theon_unimplementednote lists what is supported.IDL —
sysvar_wrappers_surface_their_idl_addresscovers the fullchain the IDL builder reads (
SysvarId::IDL_ADDRESS→IdlAccountType::__IDL_ADDRESS) for all three sysvars.Also adds the wrapper to the account-types table in
lang-v2/README.md.Branch Target
anchor-next— this is v2-only work; the files it touches do not existon
master. It is additive, not breaking:Sysvar<Clock>/Sysvar<Rent>keep working unchanged, and the relaxed bound only widenswhat
Sysvar<T>accepts.Out of scope
While adding the IDL test I noticed a pre-existing gap unrelated to this
change:
impl IdlAccountType for Box<T>(lang-v2/src/accounts/boxed.rs)propagates
__IDL_ACCOUNT_ENTRYand__IDL_TYPE_DEFbut not__IDL_ADDRESSor__IDL_IS_SIGNER, soBox<Sysvar<Clock>>,Box<Program<System>>andBox<Signer>lose that metadata in the IDL.Left untouched here; happy to file an issue or fix it separately.