Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Made geyser interface repr(c) to improve interface stability #33703

Merged
merged 2 commits into from
Oct 23, 2023
Merged

Made geyser interface repr(c) to improve interface stability #33703

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
60844db
Made geyser interface repr(c) to improve interface stability
lijunwangs Oct 16, 2023
9ea6ebd
Made enum repr(u32)
lijunwangs Oct 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions geyser-plugin-interface/src/geyser_plugin_interface.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use {
};

#[derive(Debug, Clone, PartialEq, Eq)]
#[repr(C)]
/// Information about an account being updated
pub struct ReplicaAccountInfo<'a> {
/// The Pubkey for the account
Expand Down Expand Up @@ -43,6 +44,7 @@ pub struct ReplicaAccountInfo<'a> {
}

#[derive(Debug, Clone, PartialEq, Eq)]
#[repr(C)]
/// Information about an account being updated
/// (extended with transaction signature doing this update)
pub struct ReplicaAccountInfoV2<'a> {
Expand Down Expand Up @@ -76,6 +78,7 @@ pub struct ReplicaAccountInfoV2<'a> {
}

#[derive(Debug, Clone, PartialEq, Eq)]
#[repr(C)]
/// Information about an account being updated
/// (extended with reference to transaction doing this update)
pub struct ReplicaAccountInfoV3<'a> {
Expand Down Expand Up @@ -112,6 +115,7 @@ pub struct ReplicaAccountInfoV3<'a> {
/// If there were a change to the structure of ReplicaAccountInfo,
/// there would be new enum entry for the newer version, forcing
/// plugin implementations to handle the change.
#[repr(C)]
Copy link
Contributor

Choose a reason for hiding this comment

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

we might consider setting the discriminant width on enums explicitly. they support a syntax like #[repr(C,u{8,16,32,64})] which corresponds to similar annotations in recent C++. afaik there's no such syntax in C though

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I made them repr(u32). Thanks.

Copy link
Contributor

Choose a reason for hiding this comment

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

they'd need to be repr(C, u32), but repr(C) uses u32 discriminates by default iirc. so just repr(C) would be the same.

fwiw there's some annoying lint around multiple repr in current rust. they're only valid for data-bearing enums. given that these structs are not intended to be persisted or sent over the wire, repr(C) is probably actually the least wrong choice

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried repr(C, u32), it does not compile. According to https://doc.rust-lang.org/nomicon/other-reprs.html, it should be repr(u*) format. repr(C), also:

repr(C) is equivalent to one of repr(u*) (see the next section) for fieldless enums. The chosen size is the default enum size for the target platform's C application binary interface (ABI). Note that enum representation in C is implementation defined, so this is really a "best guess". In particular, this may be incorrect when the C code of interest is compiled with certain flags.

So I think repr(u32) is more explicit. https://github.com/rust-lang/rfcs/blob/master/text/2195-really-tagged-unions.md documented more benefits on enum with fields.

pub enum ReplicaAccountInfoVersions<'a> {
V0_0_1(&'a ReplicaAccountInfo<'a>),
V0_0_2(&'a ReplicaAccountInfoV2<'a>),
Expand All @@ -120,6 +124,7 @@ pub enum ReplicaAccountInfoVersions<'a> {

/// Information about a transaction
#[derive(Clone, Debug)]
#[repr(C)]
pub struct ReplicaTransactionInfo<'a> {
/// The first signature of the transaction, used for identifying the transaction.
pub signature: &'a Signature,
Expand All @@ -136,6 +141,7 @@ pub struct ReplicaTransactionInfo<'a> {

/// Information about a transaction, including index in block
#[derive(Clone, Debug)]
#[repr(C)]
pub struct ReplicaTransactionInfoV2<'a> {
/// The first signature of the transaction, used for identifying the transaction.
pub signature: &'a Signature,
Expand All @@ -157,12 +163,14 @@ pub struct ReplicaTransactionInfoV2<'a> {
/// If there were a change to the structure of ReplicaTransactionInfo,
/// there would be new enum entry for the newer version, forcing
/// plugin implementations to handle the change.
#[repr(C)]
pub enum ReplicaTransactionInfoVersions<'a> {
V0_0_1(&'a ReplicaTransactionInfo<'a>),
V0_0_2(&'a ReplicaTransactionInfoV2<'a>),
}

#[derive(Clone, Debug)]
#[repr(C)]
pub struct ReplicaEntryInfo<'a> {
/// The slot number of the block containing this Entry
pub slot: Slot,
Expand All @@ -180,11 +188,13 @@ pub struct ReplicaEntryInfo<'a> {
/// A wrapper to future-proof ReplicaEntryInfo handling. To make a change to the structure of
/// ReplicaEntryInfo, add an new enum variant wrapping a newer version, which will force plugin
/// implementations to handle the change.
#[repr(C)]
pub enum ReplicaEntryInfoVersions<'a> {
V0_0_1(&'a ReplicaEntryInfo<'a>),
}

#[derive(Clone, Debug)]
#[repr(C)]
pub struct ReplicaBlockInfo<'a> {
pub slot: Slot,
pub blockhash: &'a str,
Expand All @@ -195,6 +205,7 @@ pub struct ReplicaBlockInfo<'a> {

/// Extending ReplicaBlockInfo by sending the executed_transaction_count.
#[derive(Clone, Debug)]
#[repr(C)]
pub struct ReplicaBlockInfoV2<'a> {
pub parent_slot: Slot,
pub parent_blockhash: &'a str,
Expand All @@ -208,6 +219,7 @@ pub struct ReplicaBlockInfoV2<'a> {

/// Extending ReplicaBlockInfo by sending the entries_count.
#[derive(Clone, Debug)]
#[repr(C)]
pub struct ReplicaBlockInfoV3<'a> {
pub parent_slot: Slot,
pub parent_blockhash: &'a str,
Expand All @@ -220,6 +232,7 @@ pub struct ReplicaBlockInfoV3<'a> {
pub entry_count: u64,
}

#[repr(C)]
pub enum ReplicaBlockInfoVersions<'a> {
V0_0_1(&'a ReplicaBlockInfo<'a>),
V0_0_2(&'a ReplicaBlockInfoV2<'a>),
Expand All @@ -228,6 +241,7 @@ pub enum ReplicaBlockInfoVersions<'a> {

/// Errors returned by plugin calls
#[derive(Error, Debug)]
#[repr(C)]
pub enum GeyserPluginError {
/// Error opening the configuration file; for example, when the file
/// is not found or when the validator process has no permission to read it.
Expand Down Expand Up @@ -258,6 +272,7 @@ pub enum GeyserPluginError {

/// The current status of a slot
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
#[repr(C)]
pub enum SlotStatus {
/// The highest slot of the heaviest fork processed by the node. Ledger state at this slot is
/// not derived from a confirmed or finalized block, but if multiple forks are present, is from
Expand Down