Skip to content

Add PrecompilesFeatureSet#85

Closed
jstarry wants to merge 2 commits intoanza-xyz:masterfrom
jstarry:precompiles-feature-set
Closed

Add PrecompilesFeatureSet#85
jstarry wants to merge 2 commits intoanza-xyz:masterfrom
jstarry:precompiles-feature-set

Conversation

@jstarry
Copy link
Copy Markdown
Contributor

@jstarry jstarry commented Mar 25, 2025

Problem

The SDK shouldn't need to be coupled to specific feature keys to enable feature gated functionality in the SDK.

Summary

Breaking change to precompile interface to enable feature gates via flags so that the SDK doesn't need to be aware of or coupled to feature keys. This requires a major version bump of the precompiles and transaction crate according to semvar rules

  • Update the Precompile struct to have an enable function. Really this should have all be based on traits :/

@jstarry jstarry requested a review from a team as a code owner March 25, 2025 21:02
@jstarry jstarry force-pushed the precompiles-feature-set branch 3 times, most recently from 1e81cee to be7593f Compare March 25, 2025 21:14
@jstarry jstarry force-pushed the precompiles-feature-set branch from be7593f to 620b555 Compare March 25, 2025 23:34
@jstarry jstarry requested review from joncinque and t-nelson March 25, 2025 23:34
@jstarry
Copy link
Copy Markdown
Contributor Author

jstarry commented Mar 25, 2025

This can wait until after the agave feature set migration is finished if y'all want but I think this will be a good change regardless because we shouldn't have a dependency on agave crates from the sdk if we can help it.

Comment thread precompiles/src/lib.rs Outdated
Co-authored-by: Illia Bobyr <illia.bobyr@gmail.com>
Copy link
Copy Markdown
Collaborator

@joncinque joncinque left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This approach certainly works!

As another option, I'd like to point out #86, which uses solana-feature-set-interface::FeatureSet, but requires redefining the feature sets within the crates that need them.

If agave-feature-set::FeatureSet adds a function to expose the underlying solana-feature-set-interface::FeatureSet, then there should be a lot less friction in integration.

The big drawback with the other approach is redefining the feature ids, which isn't an amazing solution.

@jstarry
Copy link
Copy Markdown
Contributor Author

jstarry commented Mar 26, 2025

The big drawback with the other approach is redefining the feature ids, which isn't an amazing solution.

Yeah if it weren't for the feature ids I would definitely prefer just switching the feature set interface. Coupling sdk releases to feature id's in agave is far from ideal and likely to cause a lot of headaches and potentially serious bugs.

I think there are other solutions besides this approach. @t-nelson had some ideas as well to have some sort of generic LocalFeatures struct but I wasn't super clear on that.

@jstarry
Copy link
Copy Markdown
Contributor Author

jstarry commented Mar 26, 2025

@t-nelson suggested a better approach.. just migrate agave from using solana-precompiles to agave-precompiles and deprecate any usage of solana-precompiles in the sdk. Precompile errors stay for now tho

@jstarry
Copy link
Copy Markdown
Contributor Author

jstarry commented Mar 26, 2025

I'll likely re-introduce this change in agave, it helps make ix iteration faster by doing less feature set lookups

@jstarry jstarry closed this Mar 26, 2025
@jstarry jstarry deleted the precompiles-feature-set branch February 20, 2026 14:34
grod220 pushed a commit that referenced this pull request Mar 9, 2026
Bumps [bytemuck](https://github.com/Lokathor/bytemuck) from 1.22.0 to 1.23.0.
- [Changelog](https://github.com/Lokathor/bytemuck/blob/main/changelog.md)
- [Commits](Lokathor/bytemuck@v1.22.0...v1.23.0)

---
updated-dependencies:
- dependency-name: bytemuck
  dependency-version: 1.23.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
grod220 pushed a commit that referenced this pull request Mar 16, 2026
Bumps [bytemuck](https://github.com/Lokathor/bytemuck) from 1.22.0 to 1.23.0.
- [Changelog](https://github.com/Lokathor/bytemuck/blob/main/changelog.md)
- [Commits](Lokathor/bytemuck@v1.22.0...v1.23.0)

---
updated-dependencies:
- dependency-name: bytemuck
  dependency-version: 1.23.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
grod220 pushed a commit that referenced this pull request Mar 18, 2026
Bumps [bytemuck](https://github.com/Lokathor/bytemuck) from 1.22.0 to 1.23.0.
- [Changelog](https://github.com/Lokathor/bytemuck/blob/main/changelog.md)
- [Commits](Lokathor/bytemuck@v1.22.0...v1.23.0)

---
updated-dependencies:
- dependency-name: bytemuck
  dependency-version: 1.23.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
grod220 pushed a commit that referenced this pull request Mar 19, 2026
Bumps [bytemuck](https://github.com/Lokathor/bytemuck) from 1.22.0 to 1.23.0.
- [Changelog](https://github.com/Lokathor/bytemuck/blob/main/changelog.md)
- [Commits](Lokathor/bytemuck@v1.22.0...v1.23.0)

---
updated-dependencies:
- dependency-name: bytemuck
  dependency-version: 1.23.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
grod220 pushed a commit that referenced this pull request Mar 19, 2026
Bumps [bytemuck](https://github.com/Lokathor/bytemuck) from 1.22.0 to 1.23.0.
- [Changelog](https://github.com/Lokathor/bytemuck/blob/main/changelog.md)
- [Commits](Lokathor/bytemuck@v1.22.0...v1.23.0)

---
updated-dependencies:
- dependency-name: bytemuck
  dependency-version: 1.23.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants