Feature - Adds `UpgradeableLoaderInstruction::Migrate` by Lichtso · Pull Request #4661 · anza-xyz/agave

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 · 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 <AlexanderMeissner@gmx.net>

* sdk: Bump all SDK crates for relaxed dependencies #### Problem The SDK relaxed its internal dependencies in anza-xyz/solana-sdk#27, but the monorepo is still using the pinned versions. #### Summary of changes Update all SDK crates to their next version. For almost all of them, that's v2.2.1 Otherwise, the address-lookup-table-interface crates goes to v2.2.2, and the loader-v3-interface goes to v3.0.0 due to a breaking change. To keep this PR small, the logic changes required for v3.0.0 are not implemented, and must be done in follow-up work, which essentially means reapplying the changes from #4661. * Return errors instead of panicking

* sdk: Bump all SDK crates for relaxed dependencies #### Problem The SDK relaxed its internal dependencies in anza-xyz/solana-sdk#27, but the monorepo is still using the pinned versions. #### Summary of changes Update all SDK crates to their next version. For almost all of them, that's v2.2.1 Otherwise, the address-lookup-table-interface crates goes to v2.2.2, and the loader-v3-interface goes to v3.0.0 due to a breaking change. To keep this PR small, the logic changes required for v3.0.0 are not implemented, and must be done in follow-up work, which essentially means reapplying the changes from #4661. * Return errors instead of panicking (cherry picked from commit a1ed2b1)

* Adds UpgradeableLoaderInstruction::Migrate. * Adds test_loader_v3_to_v4_migration.

* Adds UpgradeableLoaderInstruction::Migrate. * Adds test_loader_v3_to_v4_migration. (cherry picked from commit 7491134) # Conflicts: # programs/bpf_loader/src/lib.rs # transaction-status/src/parse_bpf_loader.rs

…4969) (#4972) sdk: Bump all SDK crates for relaxed dependencies (#4969) * sdk: Bump all SDK crates for relaxed dependencies #### Problem The SDK relaxed its internal dependencies in anza-xyz/solana-sdk#27, but the monorepo is still using the pinned versions. #### Summary of changes Update all SDK crates to their next version. For almost all of them, that's v2.2.1 Otherwise, the address-lookup-table-interface crates goes to v2.2.2, and the loader-v3-interface goes to v3.0.0 due to a breaking change. To keep this PR small, the logic changes required for v3.0.0 are not implemented, and must be done in follow-up work, which essentially means reapplying the changes from #4661. * Return errors instead of panicking (cherry picked from commit a1ed2b1) Co-authored-by: Jon C <me@jonc.dev>

* Adds UpgradeableLoaderInstruction::Migrate. * Adds test_loader_v3_to_v4_migration. (cherry picked from commit 7491134) # Conflicts: # programs/bpf_loader/src/lib.rs # transaction-status/src/parse_bpf_loader.rs

…(backport of #4992) (#4998) Feature - Adds `UpgradeableLoaderInstruction::Migrate` (#4992) * Adds UpgradeableLoaderInstruction::Migrate. * Adds test_loader_v3_to_v4_migration. (cherry picked from commit 7491134)

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

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

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.

joncinque mentioned this pull request Feb 13, 2025

sdk: Bump all SDK crates for relaxed dependencies #4969

Merged

mergify bot mentioned this pull request Feb 13, 2025

v2.2: sdk: Bump all SDK crates for relaxed dependencies (backport of #4969) #4972

Merged

Lichtso mentioned this pull request Feb 14, 2025

Feature - Adds UpgradeableLoaderInstruction::Migrate (#4661) #4992

Merged

Lichtso added a commit that referenced this pull request Feb 14, 2025

Feature - Adds UpgradeableLoaderInstruction::Migrate (#4661) (#4992)

7491134

* Adds UpgradeableLoaderInstruction::Migrate. * Adds test_loader_v3_to_v4_migration.

mergify bot mentioned this pull request Feb 14, 2025

v2.2: Feature - Adds UpgradeableLoaderInstruction::Migrate (#4661) (backport of #4992) #4998

Merged

mergify bot mentioned this pull request Feb 25, 2025

v2.2: CLI - loader v3 to v4 migration (backport of #4856) #5067

Merged

Lichtso added the feature-gate label Apr 24, 2025

Conversation

Lichtso commented Jan 28, 2025

Problem

Summary of Changes

Uh oh!

mergify bot commented Jan 28, 2025

Uh oh!

LucasSte left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Lichtso commented Jan 28, 2025

Uh oh!

mergify bot commented Feb 4, 2025

Uh oh!

joncinque commented Feb 4, 2025

Uh oh!

mergify bot commented Feb 5, 2025

Uh oh!

LucasSte Feb 7, 2025

Choose a reason for hiding this comment

Uh oh!

Lichtso Feb 7, 2025

Choose a reason for hiding this comment

Uh oh!

LucasSte Feb 7, 2025

Choose a reason for hiding this comment

Uh oh!

Lichtso Feb 7, 2025

Choose a reason for hiding this comment

Uh oh!

LucasSte Feb 7, 2025

Choose a reason for hiding this comment

Uh oh!

Lichtso Feb 7, 2025

Choose a reason for hiding this comment

Uh oh!

joncinque commented Feb 7, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants