fix(spl): deprecate cpi_guard#4465
Conversation
The tests invoke the anchor-spl cpi_guard_enable / cpi_guard_disable
wrappers through a CPI from the test program. Token-2022's
process_toggle_cpi_guard explicitly rejects in_cpi() calls with
CpiGuardSettingsLocked, so the wrappers cannot succeed from any
on-chain program. Remove the tests; the wrappers themselves are
deprecated in a follow-up commit.
Also drop the `import { it } from "node:test"` — it caused mocha to
report "0 passing" and not observe failures, letting the CPI Guard
tests appear to pass on CI despite failing at runtime. With it gone,
mocha's global `it` picks up all suites and actually enforces the
outcome.
|
@jamie-osec is attempting to deploy a commit to the Solana Foundation Team on Vercel. A member of the Team first needs to authorize it. |
|
Does it make sense to mark as deprecated instead of outright removal? It was never useable from CPIs (otherwise protocols could disable cpi guard before calling a possible transfer, completely bypassing the idea behind the extension). The only valid reason to keep it is so that no-one tries to re-add it You've fixed the tests as well, I'll close #4459 then. |
|
Ah thanks, didn't notice your issue/PR. Deprecation would be more semver compliant, and it allows us to give a useful error message to people who are trying to use it pointing them to the right place. For v2 we can just not port this extension across |
tiago18c
left a comment
There was a problem hiding this comment.
double checked that there are no more node:test imports.
lgtm
Token-2022's process_toggle_cpi_guard returns CpiGuardSettingsLocked whenever in_cpi() is true, which is always the case for these anchor-spl wrappers (they invoke Token-2022 via CpiContext from an on-chain program). The only workable path for toggling the guard is a client-side instruction sent directly to Token-2022, which does not need an anchor-spl wrapper. Mark the two functions and the accompanying Accounts struct as deprecated so new callers get compiler guidance.
cbc4848 to
c6c914b
Compare
Co-authored-by: acheron <98934430+acheroncrypto@users.noreply.github.com>
Closes #4458
This feature was originally added in #2789. However, the
cpi_guard_enable/disableinstructions are not usable from in a CPI (https://docs.rs/crate/spl-token-2022/10.0.0/source/src/extension/cpi_guard/processor.rs#42-44), which means an Anchor CPI wrapper is not useful.I added these tests in #4322 (cc @0xIchigo), but due to the pre-existing
import { it } from "node:test";,ts-mochawas not able to properly recognise the test failures. This test has been failing since then without marking CI as failed.Remove the broken tests, fix the incorrect import, and deprecate the feature.
cc @acheroncrypto as the original reviewer for a double-check.