Feature - Adds `UpgradeableLoaderInstruction::Migrate` #4661

Lichtso · 2025-01-28T13:00:13Z

Problem

Summary of Changes

Feature Gate Issue: https://github.com/anza-xyz/feature-gate-tracker/issues/78

mergify · 2025-01-28T13:00:48Z

The Firedancer team maintains a line-for-line reimplementation of the
native programs, and until native programs are moved to BPF, those
implementations must exactly match their Agave counterparts.
If this PR represents a change to a native program implementation (not
tests), please include a reviewer from the Firedancer team. And please
keep refactors to a minimum.

LucasSte

The CI is failing.

programs/bpf_loader/src/lib.rs

Lichtso · 2025-01-28T14:21:33Z

Yep, also still have to write a test for the new instruction.

mergify · 2025-02-04T17:30:10Z

The Firedancer team maintains a line-for-line reimplementation of the
native programs, and until native programs are moved to BPF, those
implementations must exactly match their Agave counterparts.
If this PR represents a change to a native program implementation (not
tests), please include a reviewer from the Firedancer team. And please
keep refactors to a minimum.

joncinque · 2025-02-04T20:53:42Z

This PR contains changes to the solana sdk, which will be moved to a new repo within the next week, when v2.2 is branched from master.

Please merge or close this PR as soon as possible, or re-create the sdk changes when the new repository is ready at https://github.com/anza-xyz/solana-sdk

mergify · 2025-02-05T23:20:54Z

The Firedancer team maintains a line-for-line reimplementation of the
native programs, and until native programs are moved to BPF, those
implementations must exactly match their Agave counterparts.
If this PR represents a change to a native program implementation (not
tests), please include a reviewer from the Firedancer team. And please
keep refactors to a minimum.

LucasSte

This PR adds the new instruction in the loader and modifies the CLI. Would you mind splitting those into two PRs? One for the new instruction and another one for the CLI command.

cli/src/program.rs

LucasSte · 2025-02-07T14:52:15Z

programs/bpf_loader/src/lib.rs

+            // Verify authority signature
+            if !migration_authority::check_id(&provided_authority_address)
+                && provided_authority_address
+                    != upgrade_authority_address.unwrap_or(program_address)


The SIMD mentions program signer. Is that the program address?

Check that the provided authority is either:
- the migration authority
(pubkey is 3Scf35jMNk2xXBD6areNjgMtXgp5ZspDhms8vdcbzC42)
- or the upgrade authority stored in the program data account
- or the program signer if the program is finalized, closed or
uninitialized

Yes, it means the program address is also a signer.

LucasSte · 2025-02-07T14:54:43Z

programs/bpf_loader/src/lib.rs

+            } else {
+                ic_logger_msg!(log_collector, "Invalid Program account");
+                return Err(InstructionError::InvalidAccountData);
+            }


This else is not specified in the SIMD. It is necessary here, for sure. Should it be included in the instruction description in the SIMD?

I updated the SIMD

LucasSte · 2025-02-07T15:01:18Z

runtime/src/bank/tests.rs

+                InstructionError::InvalidInstructionData,
+            )),
+        ),
+        (
+            uninitialized_programdata_account,
+            finalized_migration_transaction.clone(),
+            Err(TransactionError::InstructionError(
+                0,
+                InstructionError::InvalidInstructionData,


You checked for two error cases only. Should we check for all error possibilities?

Added ten more test cases.

joncinque · 2025-02-07T22:54:43Z

Unfortunately, this PR was merged after the v2.2 branch was cut, which means that the v2.2 crates were published without this change.

Now it's blocking the removal of the SDK from the monorepo: https://buildkite.com/anza/agave/builds/19002#0194e28d-87cd-405a-bcec-8e65cb1600a3

Can we please revert this?

We'll have to apply the changes to the loader-v3 interface crate at https://github.com/anza-xyz/solana-sdk/tree/master/loader-v3-interface, publish a new major version, and then upgrade agave to use that new crate.

#### Problem The SDK changes from anza-xyz/agave#4661 need to be applied here. See [SIMD-0167](solana-foundation/solana-improvement-documents#167). #### Summary of Changes Feature Gate Issue: anza-xyz/feature-gate-tracker#78

…" (#4870) This reverts commit 11dfda5.

* loader-v3: Add `UpgradeableLoaderInstruction::Migrate` #### Problem The SDK changes from anza-xyz/agave#4661 need to be applied here. See [SIMD-0167](solana-foundation/solana-improvement-documents#167). #### Summary of Changes Feature Gate Issue: anza-xyz/feature-gate-tracker#78 * Avoid change in solana-program --------- Co-authored-by: Alexander Meißner <[email protected]>

Lichtso requested a review from a team as a code owner January 28, 2025 13:00

Lichtso force-pushed the feature/loader_v3_to_v4_migration branch 2 times, most recently from ba4a1ef to d9db125 Compare January 28, 2025 13:17

LucasSte reviewed Jan 28, 2025

View reviewed changes

programs/bpf_loader/src/lib.rs Outdated Show resolved Hide resolved

Lichtso force-pushed the feature/loader_v3_to_v4_migration branch from d9db125 to c526f4d Compare January 28, 2025 14:16

Lichtso marked this pull request as draft January 28, 2025 14:20

Lichtso force-pushed the feature/loader_v3_to_v4_migration branch 5 times, most recently from a5b2de4 to a141a1a Compare January 29, 2025 15:47

Lichtso mentioned this pull request Jan 29, 2025

Refactor - loader v4 SetProgramLength #4687

Merged

Lichtso force-pushed the feature/loader_v3_to_v4_migration branch 5 times, most recently from 2ccc774 to e5f2665 Compare February 3, 2025 16:36

Lichtso force-pushed the feature/loader_v3_to_v4_migration branch 5 times, most recently from 538e50e to ae58ed5 Compare February 5, 2025 23:20

Lichtso force-pushed the feature/loader_v3_to_v4_migration branch 3 times, most recently from 2f9e218 to 7d0f4d0 Compare February 6, 2025 21:09

Lichtso marked this pull request as ready for review February 6, 2025 23:13

Lichtso requested a review from pgarg66 February 6, 2025 23:13

LucasSte reviewed Feb 7, 2025

View reviewed changes

cli/src/program.rs Outdated Show resolved Hide resolved

Lichtso force-pushed the feature/loader_v3_to_v4_migration branch from 7d0f4d0 to 68c8f8f Compare February 7, 2025 14:38

Lichtso mentioned this pull request Feb 7, 2025

CLI - loader v3 to v4 migration #4856

Open

Adds UpgradeableLoaderInstruction::Migrate.

af0e909

Lichtso force-pushed the feature/loader_v3_to_v4_migration branch from 68c8f8f to 48f11f4 Compare February 7, 2025 14:43

LucasSte reviewed Feb 7, 2025

View reviewed changes

Adds test_loader_v3_to_v4_migration.

66974b0

Lichtso force-pushed the feature/loader_v3_to_v4_migration branch from 48f11f4 to 66974b0 Compare February 7, 2025 18:19

LucasSte approved these changes Feb 7, 2025

View reviewed changes

Lichtso merged commit 11dfda5 into anza-xyz:master Feb 7, 2025
77 checks passed

Lichtso deleted the feature/loader_v3_to_v4_migration branch February 7, 2025 22:19

This was referenced Feb 7, 2025

sdk: Remove everything from the repo #4867

Merged

Revert "Feature - Adds UpgradeableLoaderInstruction::Migrate (#4661)" #4870

Merged

joncinque mentioned this pull request Feb 7, 2025

loader-v3: Add UpgradeableLoaderInstruction::Migrate anza-xyz/solana-sdk#9

Merged

joncinque added a commit that referenced this pull request Feb 8, 2025

Revert "Feature - Adds UpgradeableLoaderInstruction::Migrate (#4661)…

3bbe4ca

…" (#4870) This reverts commit 11dfda5.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature - Adds `UpgradeableLoaderInstruction::Migrate` #4661

Feature - Adds `UpgradeableLoaderInstruction::Migrate` #4661

Lichtso commented Jan 28, 2025

mergify bot commented Jan 28, 2025

LucasSte left a comment

Lichtso commented Jan 28, 2025

mergify bot commented Feb 4, 2025

joncinque commented Feb 4, 2025

mergify bot commented Feb 5, 2025

LucasSte left a comment

LucasSte Feb 7, 2025

Lichtso Feb 7, 2025

LucasSte Feb 7, 2025

Lichtso Feb 7, 2025

LucasSte Feb 7, 2025

Lichtso Feb 7, 2025

joncinque commented Feb 7, 2025

Feature - Adds UpgradeableLoaderInstruction::Migrate #4661

Feature - Adds UpgradeableLoaderInstruction::Migrate #4661

Conversation

Lichtso commented Jan 28, 2025

Problem

Summary of Changes

mergify bot commented Jan 28, 2025

LucasSte left a comment

Choose a reason for hiding this comment

Lichtso commented Jan 28, 2025

mergify bot commented Feb 4, 2025

joncinque commented Feb 4, 2025

mergify bot commented Feb 5, 2025

LucasSte left a comment

Choose a reason for hiding this comment

LucasSte Feb 7, 2025

Choose a reason for hiding this comment

Lichtso Feb 7, 2025

Choose a reason for hiding this comment

LucasSte Feb 7, 2025

Choose a reason for hiding this comment

Lichtso Feb 7, 2025

Choose a reason for hiding this comment

LucasSte Feb 7, 2025

Choose a reason for hiding this comment

Lichtso Feb 7, 2025

Choose a reason for hiding this comment

joncinque commented Feb 7, 2025

Feature - Adds `UpgradeableLoaderInstruction::Migrate` #4661

Feature - Adds `UpgradeableLoaderInstruction::Migrate` #4661