v2.1: Fix builtin default cost dependents on migration (backport of #3768)

[mergify]

Problem

Some of builtin programs are being migrated to sbpf, while the rest is being planned for too. Once a builtin program is migrated in a cluster, it's "default cost" defined in BUILTIN_INSTRUCTION_COSTS should be changed to 0.

Summary of Changes

• add public function to fetch correct default cost for builtin program, based on migration feature status
• add BuiltinCost struct to track migration feature ID
• add developer notes to remind to add migration feature ID whenever it becomes available.

Fixes #

---

This is an automatic backport of pull request #3768 done by [Mergify](https://mergify.com).

[mergify]

Cherry-pick of d791c9a has failed:

On branch mergify/bp/v2.1/pr-3768
Your branch is up to date with 'origin/v2.1'.

You are currently cherry-picking commit d791c9a76.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   builtins-default-costs/benches/builtin_instruction_costs.rs
	modified:   core/src/banking_stage/packet_filter.rs
	modified:   runtime/src/bank/builtins/mod.rs

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   builtins-default-costs/src/lib.rs
	both modified:   cost-model/src/cost_model.rs

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

[tao-stones]

A prerequisite bp for backporting #3975 to 2.1 (#4091).

Details:

• this, Fix builtin default cost dependents on migration #3768, introduced pub fn get_builtin_instruction_cost(&Pubkey, &FeatureSet) for migration-featuree-awared method of getting builtin program cost (eg: Some(native_cost) for builtins, or Null for migrated-builtin/non-builtins)
• next, Accounting migrating builtin programs default Compute Unit Limit with feature status #3975, built on top of that to set correct default Compute Budget Limit (eg 3K or 200K) for builtin programs based on their migration status.

[jstarry]

For the record, I don't think the get_builtin_instruction_cost function is actually needed in v2.1. As far as I can tell, it's only used in packet filtering and in get_transaction_cost_without_minimal_builtin_cus both of which aren't important for replay consistency. But we do need the changes to the BUILTIN_INSTRUCTION_COSTS map which was also made "migration feature" aware in #3768

[tao-stones]

cost_model also calls get_builtin_instruction_cost function to get builtin cost with feature status. But yea, the main change is to the BUILTIN_INSTRUCTION_COSTS map and demoting its visibility to private, introducing get_builtin_instruction_cost function as public api.

[jstarry]

cost_model also calls get_builtin_instruction_cost function to get builtin cost with feature status

Yeah, only in get_transaction_cost_without_minimal_builtin_cus right? That code path isn't really critical.

[tao-stones]

right, it's the execution path before feature reserve_minimal_cus_for_builtin_instructions is activated; after activation, it'd be obsoleted.

[t-nelson]

is #4091 not smuggling this change set? 4091 looks nothing like the master pr

[t-nelson]

seems like an awfully roundabout way to get the job done 😭

[t-nelson]

discarding pub breaks api here. probably what we eventually want, but not the way to do it

[t-nelson]

discarding pub breaks api here. probably what we eventually want, but not the way to do it

then again the signature is wrecked too, sigh. at least any brain dead consumers can adapt if it remains pub

[jstarry]

Why are you concerned with API compatibility in this crate?

[t-nelson]

why can't we just resolve the feature gate status at a higher level and use a bool instead of some poorly named sentinel value?

[jstarry]

Is this important for backport review? Also can you explain the bool you're talking about? I'm not getting what you're suggesting

[tao-stones]

At the receiving & buffer part of Banking_stage, resolving feature gate status is always bit fuzzy since we have a 'working_bank' yet. There are certainly ways around it, but nothing is clean and easy afaik.

[tao-stones]

is #4091 not smuggling this change set? 4091 looks nothing like the master pr

The urge to sneak it into #4091 was strong, but we decided to do it right: #4091 (comment). With this, 4091 looks pretty close to its original PR #3975