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.

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

[Solana][readability] PluginType type safety #631

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
33 changes: 32 additions & 1 deletion chains/solana/contracts/programs/ccip-offramp/src/context.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use anchor_lang::prelude::*;
use bytemuck::{Pod, Zeroable};
use solana_program::sysvar::instructions;

use crate::messages::ExecutionReportSingleChain;
Expand Down Expand Up @@ -518,7 +519,37 @@ pub struct ExecuteReportContext<'info> {
// ] x N tokens
}

#[derive(Copy, Clone, AnchorSerialize, AnchorDeserialize)]
/// It's not possible to store enums in zero_copy accounts, so we wrap the discriminant
/// in a struct to store in config.
#[derive(
Copy, Clone, AnchorSerialize, AnchorDeserialize, PartialEq, Eq, InitSpace, Pod, Zeroable,
)]
#[repr(C)]
pub struct ConfigOcrPluginType {
discriminant: u8,
}

impl From<OcrPluginType> for ConfigOcrPluginType {
fn from(value: OcrPluginType) -> Self {
Self {
discriminant: value as u8,
}
}
}

impl TryFrom<ConfigOcrPluginType> for OcrPluginType {
type Error = CcipOfframpError;

fn try_from(value: ConfigOcrPluginType) -> std::result::Result<Self, Self::Error> {
match value.discriminant {
0 => Ok(Self::Commit),
1 => Ok(Self::Execution),
_ => Err(CcipOfframpError::InvalidPluginType),
}
}
}

#[derive(Copy, Clone, AnchorSerialize, AnchorDeserialize, PartialEq, Eq)]
pub enum OcrPluginType {
Commit,
Execution,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ use crate::context::{
PriceOnlyCommitReportContext, SetOcrConfig, TransferOwnership, UpdateConfig, UpdateSourceChain,
};
use crate::state::{CodeVersion, Ocr3ConfigInfo, SourceChainConfig};
use crate::OcrPluginType;

/// To be called by the commit DON.
pub trait Commit {
fn commit<'info>(
&self,
Expand All @@ -28,6 +30,7 @@ pub trait Commit {
) -> Result<()>;
}

/// To be called by the execute DON.
pub trait Execute {
fn execute<'info>(
&self,
Expand All @@ -45,6 +48,7 @@ pub trait Execute {
) -> Result<()>;
}

/// To be called by the offramp administrator.
pub trait Admin {
fn transfer_ownership(
&self,
Expand Down Expand Up @@ -95,7 +99,7 @@ pub trait Admin {
fn set_ocr_config(
&self,
ctx: Context<SetOcrConfig>,
plugin_type: u8, // OcrPluginType, u8 used because anchor tests did not work with an enum
plugin_type: OcrPluginType,
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this a breaking change? If yes, then we need to inform tooling team to update their interactions as well.

config_info: Ocr3ConfigInfo,
signers: Vec<[u8; 20]>,
transmitters: Vec<Pubkey>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,15 +154,14 @@ impl Admin for Impl {
fn set_ocr_config(
&self,
ctx: Context<SetOcrConfig>,
plugin_type: u8, // OcrPluginType, u8 used because anchor tests did not work with an enum
plugin_type: OcrPluginType,
config_info: Ocr3ConfigInfo,
signers: Vec<[u8; 20]>,
transmitters: Vec<Pubkey>,
) -> Result<()> {
require!(plugin_type < 2, CcipOfframpError::InvalidPluginType);
let mut config = ctx.accounts.config.load_mut()?;

let is_commit = plugin_type == OcrPluginType::Commit as u8;
let is_commit = plugin_type == OcrPluginType::Commit;

ocr3_set(
&mut config.ocr3[plugin_type as usize],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ impl Commit for Impl {
&config.ocr3[OcrPluginType::Commit as usize],
&ctx.accounts.sysvar_instructions,
ctx.accounts.authority.key(),
OcrPluginType::Commit as u8,
OcrPluginType::Commit,
report_context,
&Ocr3ReportForCommit(&report),
Signatures { rs, ss, raw_vs },
Expand Down Expand Up @@ -206,7 +206,7 @@ impl Commit for Impl {
&config.ocr3[OcrPluginType::Commit as usize],
&ctx.accounts.sysvar_instructions,
ctx.accounts.authority.key(),
OcrPluginType::Commit as u8,
OcrPluginType::Commit,
report_context,
&Ocr3ReportForCommit(&report),
Signatures { rs, ss, raw_vs },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ impl Execute for Impl {
&config.ocr3[OcrPluginType::Execution as usize],
&ctx.accounts.sysvar_instructions,
ctx.accounts.authority.key(),
OcrPluginType::Execution as u8,
OcrPluginType::Execution,
report_context,
&Ocr3ReportForExecutionReportSingleChain(&execution_report),
Signatures {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use anchor_lang::solana_program::{keccak, secp256k1_recover::*};

use crate::ocr3base::{ConfigSet, Ocr3Error, Transmitted, MAX_ORACLES};
use crate::state::{Ocr3Config, Ocr3ConfigInfo};
use crate::OcrPluginType;

pub const MAX_SIGNERS: usize = MAX_ORACLES;
pub const MAX_TRANSMITTERS: usize = MAX_ORACLES;
Expand Down Expand Up @@ -48,13 +49,13 @@ impl ReportContext {

pub fn ocr3_set(
ocr3_config: &mut Ocr3Config,
plugin_type: u8,
plugin_type: OcrPluginType,
cfg: Ocr3ConfigInfo,
signers: Vec<[u8; 20]>,
transmitters: Vec<Pubkey>,
) -> Result<()> {
require!(
plugin_type == ocr3_config.plugin_type,
plugin_type == ocr3_config.plugin_type.try_into()?,
Ocr3Error::InvalidPluginType
);
require!(cfg.f != 0, Ocr3Error::InvalidConfigFMustBePositive);
Expand Down Expand Up @@ -101,7 +102,7 @@ pub fn ocr3_set(
ocr3_config.config_info.config_digest = cfg.config_digest;

emit!(ConfigSet {
ocr_plugin_type: ocr3_config.plugin_type,
ocr_plugin_type: ocr3_config.plugin_type.try_into()?,
config_digest: ocr3_config.config_info.config_digest,
signers,
transmitters,
Expand All @@ -128,13 +129,13 @@ pub(super) fn ocr3_transmit<R: Ocr3Report>(
ocr3_config: &Ocr3Config,
instruction_sysvar: &AccountInfo<'_>,
transmitter: Pubkey,
plugin_type: u8,
plugin_type: OcrPluginType,
report_context: ReportContext,
report: &R,
signatures: Signatures,
) -> Result<()> {
require!(
plugin_type == ocr3_config.plugin_type,
plugin_type == ocr3_config.plugin_type.try_into()?,
Ocr3Error::InvalidPluginType
);

Expand Down Expand Up @@ -189,7 +190,7 @@ pub(super) fn ocr3_transmit<R: Ocr3Report>(
}

emit!(Transmitted {
ocr_plugin_type: ocr3_config.plugin_type,
ocr_plugin_type: ocr3_config.plugin_type.try_into()?,
config_digest: ocr3_config.config_info.config_digest,
sequence_number: report_context.sequence_number(),
});
Expand Down
6 changes: 3 additions & 3 deletions chains/solana/contracts/programs/ccip-offramp/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,8 @@ pub mod ccip_offramp {
config.enable_manual_execution_after = enable_execution_after;
config.owner = ctx.accounts.authority.key();
config.ocr3 = [
Ocr3Config::new(OcrPluginType::Commit as u8),
Ocr3Config::new(OcrPluginType::Execution as u8),
Ocr3Config::new(OcrPluginType::Commit),
Ocr3Config::new(OcrPluginType::Execution),
];

emit!(ConfigSet {
Expand Down Expand Up @@ -292,7 +292,7 @@ pub mod ccip_offramp {
/// * `transmitters` - The list of transmitters.
pub fn set_ocr_config(
ctx: Context<SetOcrConfig>,
plugin_type: u8, // OcrPluginType, u8 used because anchor tests did not work with an enum
plugin_type: OcrPluginType,
config_info: Ocr3ConfigInfo,
signers: Vec<[u8; 20]>,
transmitters: Vec<Pubkey>,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use anchor_lang::prelude::*;

use crate::OcrPluginType;

#[constant]
pub const MAX_ORACLES: usize = 16; // can set a maximum of 16 transmitters + 16 signers simultaneously in a single set config tx

Expand Down Expand Up @@ -41,7 +43,7 @@ pub enum Ocr3Error {

#[event]
pub struct ConfigSet {
pub ocr_plugin_type: u8,
pub ocr_plugin_type: OcrPluginType,
pub config_digest: [u8; 32],
pub signers: Vec<[u8; 20]>,
pub transmitters: Vec<Pubkey>,
Expand All @@ -50,7 +52,7 @@ pub struct ConfigSet {

#[event]
pub struct Transmitted {
pub ocr_plugin_type: u8,
pub ocr_plugin_type: OcrPluginType,
pub config_digest: [u8; 32],
pub sequence_number: u64,
}
21 changes: 16 additions & 5 deletions chains/solana/contracts/programs/ccip-offramp/src/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::fmt::Display;
use anchor_lang::prelude::borsh::{BorshDeserialize, BorshSerialize};
use anchor_lang::prelude::*;

use crate::CcipOfframpError;
use crate::{CcipOfframpError, ConfigOcrPluginType, OcrPluginType};

// zero_copy is used to prevent hitting stack/heap memory limits
#[account(zero_copy)]
Expand Down Expand Up @@ -45,19 +45,30 @@ pub struct Ocr3ConfigInfo {
// signers: pubkey is 20-byte address, secp256k1 curve ECDSA
// transmitters: 32-byte pubkey, ed25519

#[derive(AnchorSerialize, AnchorDeserialize, InitSpace)]
#[zero_copy]
#[derive(AnchorSerialize, AnchorDeserialize, InitSpace, Default)]
pub struct Ocr3Config {
pub plugin_type: u8, // plugin identifier for validation (example: ccip:commit = 0, ccip:execute = 1)
pub plugin_type: ConfigOcrPluginType, // plugin identifier for validation (example: ccip:commit = 0, ccip:execute = 1)
pub config_info: Ocr3ConfigInfo,
pub signers: [[u8; 20]; 16], // v0.29.0 - anchor IDL does not build with MAX_SIGNERS
pub transmitters: [[u8; 32]; 16], // v0.29.0 - anchor IDL does not build with MAX_TRANSMITTERS
}

impl Default for Ocr3Config {
fn default() -> Self {
Self {
plugin_type: OcrPluginType::Commit.into(),
config_info: Default::default(),
signers: Default::default(),
transmitters: Default::default(),
}
}
}

impl Ocr3Config {
pub fn new(plugin_type: u8) -> Self {
pub fn new(plugin_type: OcrPluginType) -> Self {
Self {
plugin_type,
plugin_type: plugin_type.into(),
..Default::default()
}
}
Expand Down
32 changes: 28 additions & 4 deletions chains/solana/contracts/target/idl/ccip_offramp.json
Original file line number Diff line number Diff line change
Expand Up @@ -452,7 +452,9 @@
"args": [
{
"name": "pluginType",
"type": "u8"
"type": {
"defined": "OcrPluginType"
}
},
{
"name": "configInfo",
Expand Down Expand Up @@ -1271,6 +1273,22 @@
]
}
},
{
"name": "ConfigOcrPluginType",
"docs": [
"It's not possible to store enums in zero_copy accounts, so we wrap the discriminant",
"in a struct to store in config."
],
"type": {
"kind": "struct",
"fields": [
{
"name": "discriminant",
"type": "u8"
}
]
}
},
{
"name": "ExecutionReportSingleChain",
"docs": [
Expand Down Expand Up @@ -1510,7 +1528,9 @@
"fields": [
{
"name": "pluginType",
"type": "u8"
"type": {
"defined": "ConfigOcrPluginType"
}
},
{
"name": "configInfo",
Expand Down Expand Up @@ -1942,7 +1962,9 @@
"fields": [
{
"name": "ocrPluginType",
"type": "u8",
"type": {
"defined": "OcrPluginType"
},
"index": false
},
{
Expand Down Expand Up @@ -1986,7 +2008,9 @@
"fields": [
{
"name": "ocrPluginType",
"type": "u8",
"type": {
"defined": "OcrPluginType"
},
"index": false
},
{
Expand Down
Loading
Loading