Clean up feature flags#180
Conversation
SC4RECOIN
requested review from
Oighty and
Copilot
and removed request for
Copilot
There was a problem hiding this comment.
Pull request overview
This PR cleans up feature-flag usage across the Earn program and related tooling, removing migration-specific code paths and aligning build/config defaults.
Changes:
- Removed the
migrate-gatedinitializeentrypoint and simplifiedinitializelogic to always use the providedcurrent_index. - Updated Earn crate features (removed
migrate/devnet/mainnet, cleared default features) and adjusted CI build invocation for Earn. - Fixed the CLI’s Portal PDA seed for mint authority from
token_authoritytoauthority.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| services/cli/main.ts | Updates Portal authority PDA seed used for mint authority derivation. |
| programs/earn/src/lib.rs | Removes migrate-only initialize overload; initialize always takes current_index. |
| programs/earn/src/instructions/admin/initialize.rs | Deletes migrate-only accounts/logic and always updates multiplier from _current_index. |
| programs/earn/Cargo.toml | Removes feature flags and clears default features. |
| .github/setup/action.yml | Updates build steps formatting and builds Earn with explicit testing feature + no defaults. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| no-idl = [] | ||
| no-log-ix-name = [] | ||
| release = [] | ||
| testing = [] |
There was a problem hiding this comment.
This PR removes the migrate, devnet, and mainnet features and also clears the default feature set. However, the repo still references these features in build tooling (e.g., Makefile builds --features migrate,testing and upgrade targets use --features devnet/mainnet), which will fail with "unknown feature" errors. Please update the build scripts/docs/CI to stop using the removed features (or keep the features until those references are removed).
| testing = [] | |
| testing = [] | |
| migrate = [] | |
| devnet = [] | |
| mainnet = [] |
| - name: Build test programs | ||
| run: | | ||
| make build-test-earn-programs | ||
| run: make build-test-earn-programs | ||
| shell: bash |
There was a problem hiding this comment.
CI still runs make build-test-earn-programs, but that Makefile target currently builds an earn variant with --features migrate,testing. Since this PR removes the migrate feature from programs/earn/Cargo.toml, this step will fail unless the Makefile target (and any related tests expecting the migrate build) is updated accordingly.
| // Verify that the new multiplier is less than or equal to the current index (if migrating) or provided index (if not migrating) | ||
| // This is required because the call to our update_multiplier fn will fail silently if the multiplier on the mint is greater. | ||
| // That behavior is desired except when initializing the program. Therefore, we catch the error here. | ||
| let current_multiplier: f64; | ||
| let mint_multiplier: f64 = scaled_ui_config.new_multiplier.into(); | ||
| cfg_if! { | ||
| if #[cfg(feature = "migrate")] { | ||
| current_multiplier = self.old_global_account.index as f64 / INDEX_SCALE_F64; | ||
| let current_multiplier = _current_index as f64 / INDEX_SCALE_F64; |
There was a problem hiding this comment.
The comment above the multiplier check still references migration logic ("if migrating" / "if not migrating"), but migration support has been removed from this instruction. Please update the comment to match the current behavior (it now always compares against the provided current_index).
| } | ||
|
|
||
| #[cfg(not(feature = "migrate"))] | ||
| pub fn initialize(ctx: Context<Initialize>, current_index: u64) -> Result<()> { |
There was a problem hiding this comment.
initialize now always requires a current_index argument. There are existing TS callers in the repo (e.g. tests/unit/earn.test.ts) that still call .initialize() with no args, which will break builds/tests once the IDL updates. Either update/remove those callers (and any migration-only flows), or reintroduce a backward-compatible instruction interface (e.g., accept Option<u64> and handle None).
| pub fn initialize(ctx: Context<Initialize>, current_index: u64) -> Result<()> { | |
| pub fn initialize(ctx: Context<Initialize>, current_index: Option<u64>) -> Result<()> { | |
| let current_index = current_index.unwrap_or(0); |
No description provided.