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

added method to form message using chunks #165

Closed
wants to merge 6 commits into from
Closed
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
4 changes: 3 additions & 1 deletion solana/solana-ibc/programs/solana-ibc/src/ibc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@ pub use ibc::core::client::context::client_state::{
pub use ibc::core::client::context::consensus_state::ConsensusState;
pub use ibc::core::client::context::types::error::ClientError;
#[cfg(test)]
pub use ibc::core::client::context::types::msgs::{ClientMsg, MsgCreateClient};
pub use ibc::core::client::context::types::msgs::{
ClientMsg, MsgCreateClient, MsgUpdateClient,
};
pub use ibc::core::client::context::{
ClientExecutionContext, ClientValidationContext,
};
Expand Down
102 changes: 99 additions & 3 deletions solana/solana-ibc/programs/solana-ibc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,16 @@ use borsh::BorshDeserialize;
use storage::TransferAccounts;
use trie_ids::PortChannelPK;

use crate::ibc::{ClientStateValidation, SendPacketValidationContext};
use crate::ibc::{
ClientError, ClientStateValidation, SendPacketValidationContext,
};

pub const CHAIN_SEED: &[u8] = b"chain";
pub const PACKET_SEED: &[u8] = b"packet";
pub const SOLANA_IBC_STORAGE_SEED: &[u8] = b"private";
pub const TRIE_SEED: &[u8] = b"trie";
pub const MINT_ESCROW_SEED: &[u8] = b"mint_escrow";
pub const MSG_CHUNKS: &[u8] = b"msg_chunks";

declare_id!("EnfDJsAK7BGgetnmKzBx86CsgC5kfSPcsktFCQ4YLC81");

Expand All @@ -42,9 +45,8 @@ mod validation_context;
#[anchor_lang::program]
pub mod solana_ibc {

use ::ibc::core::client::types::error::ClientError;

use super::*;
use crate::storage::MsgChunks;

/// Initialises the guest blockchain with given configuration and genesis
/// epoch.
Expand Down Expand Up @@ -144,6 +146,23 @@ pub mod solana_ibc {
.map_err(move |err| error!((&err)))
}

pub fn deliver_with_chunks<'a, 'info>(
ctx: Context<'a, 'a, 'a, 'info, DeliverWithChunks<'info>>,
) -> Result<()> {
let msg_chunks = &ctx.accounts.msg_chunks;
let cloned_msg_chunks: &mut MsgChunks = &mut msg_chunks.clone();
let mut store = storage::from_ctx!(ctx, with accounts);
let mut router = store.clone();

let message = ibc::MsgEnvelope::try_from(ibc::Any::from(
cloned_msg_chunks.clone(),
))
Comment on lines +153 to +159
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we clone msg_chunks twice? Why is cloned_msg_chunks an exclusive reference?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

its a reference because i needed msg_chunks as MsgChunks so that i could convert it. But i think we can ignore it as we are doing the deserializing stuff in the form_msg_chunks method itself. So we would just be fetching the MsgEnvelope from the account and hence no conversions would be required in the deliver_chunks method.

.unwrap();
::ibc::core::entrypoint::dispatch(&mut store, &mut router, message)
.map_err(error::Error::ContextError)
.map_err(move |err| error!((&err)))
}

/// Called to set up escrow and mint accounts for given channel and denom.
/// Panics if called without `mocks` feature.
pub fn mock_init_escrow<'a, 'info>(
Expand Down Expand Up @@ -263,6 +282,27 @@ pub mod solana_ibc {
.map_err(error::Error::ContextError)
.map_err(|err| error!((&err)))
}

/// Store messages which are divided into chunk in an account which can be accessed later
/// from the deliver method.
///
/// Since solana programs have an instruction limit of 1232 bytes, we cannot send arguments
/// with large data. So we divide the data into chunks, call the method below and add it to
/// the account at the specified offset.
pub fn form_msg_chunks(
ctx: Context<FormMessageChunks>,
type_url: String,
total_len: u32,
offset: u32,
bytes: Vec<u8>,
) -> Result<()> {
let store = &mut ctx.accounts.msg_chunks;
if store.value.is_empty() {
store.new_alloc(total_len as usize, type_url);
}
store.copy_into(offset.try_into().unwrap(), &bytes);
Ok(())
}
}

/// All the storage accounts are initialized here since it is only called once
Expand Down Expand Up @@ -408,6 +448,49 @@ pub struct Deliver<'info> {
system_program: Program<'info, System>,
}

#[derive(Accounts, Clone)]
pub struct DeliverWithChunks<'info> {
#[account(mut)]
sender: Signer<'info>,

receiver: Option<AccountInfo<'info>>,

/// The account holding private IBC storage.
#[account(mut,seeds = [SOLANA_IBC_STORAGE_SEED],
bump)]
storage: Account<'info, storage::PrivateStorage>,

#[account(mut, close = sender)]
msg_chunks: Box<Account<'info, storage::MsgChunks>>,

/// The account holding provable IBC storage, i.e. the trie.
///
/// CHECK: Account’s owner is checked by [`storage::get_provable_from`]
/// function.
#[account(mut, seeds = [TRIE_SEED],
bump)]
trie: UncheckedAccount<'info>,

/// The guest blockchain data.
#[account(mut, seeds = [CHAIN_SEED], bump)]
chain: Box<Account<'info, chain::ChainData>>,
#[account(mut, seeds = [MINT_ESCROW_SEED], bump)]
/// CHECK:
mint_authority: Option<UncheckedAccount<'info>>,
#[account(mut, mint::decimals = 6, mint::authority = mint_authority)]
token_mint: Option<Box<Account<'info, Mint>>>,
#[account(mut, token::mint = token_mint, token::authority = mint_authority)]
escrow_account: Option<Box<Account<'info, TokenAccount>>>,
#[account(init_if_needed, payer = sender,
associated_token::mint = token_mint,
associated_token::authority = receiver)]
receiver_token_account: Option<Box<Account<'info, TokenAccount>>>,

associated_token_program: Option<Program<'info, AssociatedToken>>,
token_program: Option<Program<'info, Token>>,
system_program: Program<'info, System>,
}

#[derive(Accounts)]
#[instruction(port_id: ibc::PortId, channel_id_on_b: ibc::ChannelId, base_denom: String)]
pub struct MockInitEscrow<'info> {
Expand Down Expand Up @@ -505,6 +588,19 @@ pub struct SendPacket<'info> {
system_program: Program<'info, System>,
}

#[derive(Accounts)]
pub struct FormMessageChunks<'info> {
#[account(mut)]
sender: Signer<'info>,

#[account(init_if_needed, payer = sender, seeds = [MSG_CHUNKS], bump, space = 10240)]
pub msg_chunks: Account<'info, storage::MsgChunks>,
Comment on lines +596 to +597
Copy link
Collaborator

Choose a reason for hiding this comment

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

Two things.

First of all, do we need this to be a PDA? I was actually wondering if it wouldn’t be better if this was just an account owned by the sender and then they can manage it in any way they want. All we need here is a writeable account.

Second of all, if this is a PDA it must include sender in the seeds. Without that, users will overwrite each others calls.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes i was thinking about it too. I was also looking if we could create an account with data outside of smart contract. Couldnt find anything as now. But we dont need PDA, we could use the sender's account too. Once the deliver_chunks is called, we can also close the account so that we can return the rent.


pub system_program: Program<'info, System>,
}



impl ibc::Router for storage::IbcStorage<'_, '_> {
//
fn get_route(&self, module_id: &ibc::ModuleId) -> Option<&dyn ibc::Module> {
Expand Down
34 changes: 34 additions & 0 deletions solana/solana-ibc/programs/solana-ibc/src/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,40 @@ pub(crate) struct IbcStorageInner<'a, 'b> {
pub chain: &'a mut crate::chain::ChainData,
}

/// Struct containing message chunks
///
/// The struct consists of chunks of message data having the first 4 bytes
/// indicating the total size of the message
#[account]
#[derive(Debug)]
pub struct MsgChunks {
pub type_url: String,
pub value: Vec<u8>,
}
Comment on lines +382 to +391
Copy link
Collaborator

Choose a reason for hiding this comment

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

This really should be zero copy or unchecked account IMO. Furthermore, rather than giving it a structure with type url and value, it’s better to treat it as a length-prefixed sequence of bytes. Those can than be decoded as proto to get Any message.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

the value is length prefixed but we would still need type_url right?

Copy link
Collaborator

Choose a reason for hiding this comment

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

No. The value would be an Any proto message so it would include type url.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh i see, that makes sense.


impl MsgChunks {
/// Creates a new msg vector of size `total_length + 4` with 0s where the
/// first 4 bytes are allocated for the total size of the message
pub fn new_alloc(&mut self, total_len: usize, type_url: String) {
let msg = vec![0; total_len + 4];
self.value = msg;
self.type_url = type_url;
let total_len_in_bytes = (total_len as u32).to_be_bytes();
self.copy_into(0, &total_len_in_bytes);
}

pub fn copy_into(&mut self, position: usize, data: &[u8]) {
msg!("data size -> {} {}", data.len(), self.value.len());
self.value[position..position + data.len()].copy_from_slice(data);
}
}

impl From<MsgChunks> for ibc::Any {
fn from(value: MsgChunks) -> Self {
ibc::Any { type_url: value.type_url, value: value.value[4..].to_vec() }
}
}

/// A reference-counted reference to the IBC storage.
///
/// Uses inner-mutability via [`RefCell`] to allow modifications to the storage.
Expand Down
Loading
Loading