feat: add advanced patterns and standards - #3
Conversation
New comprehensive guide covering: - Factory pattern and deterministic deployment - Upgradeable contracts (SEP-0049): versioning, migrations, atomic upgrades - Governance patterns: time-locks, multi-sig with proposals/approvals - DeFi patterns: vault (SEP-0056), oracle integration, AMM liquidity pools - Resource optimization: batching, storage optimization, lazy loading - Compliance patterns (SEP-0057): whitelisting, transfer restrictions, clawback - Cross-contract callback patterns
Comprehensive reference for Stellar standards: - Contract SEPs: SEP-0041 (tokens), SEP-0046 (metadata), SEP-0048 (interface spec), SEP-0049 (upgrades), SEP-0050 (NFTs), SEP-0056 (vaults), SEP-0057 (regulated tokens) - CAPs organized by protocol version (20-25) - Quick reference tables by use case (tokens, DeFi, smart wallets, lifecycle) - Implementation examples with correct CLI syntax - Ecosystem SEPs for authentication, anchors, and compliance
Documents Protocol 22+ constructor feature: - __constructor function definition and rules - Deployment with constructor args (CLI, contract, JS SDK) - Constructor vs initialize pattern comparison table - Atomic initialization guarantees
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive documentation for advanced Soroban development patterns and a complete SEP/CAP standards reference, addressing critical gaps in the skill documentation. The additions provide production-ready code patterns for complex use cases like contract upgrades, governance, DeFi primitives, and compliance features.
Changes:
- Added
advanced-patterns.mdwith factory patterns, upgradeable contracts (SEP-0049), governance/multi-sig patterns, DeFi primitives (vaults, oracles, AMM), optimization strategies, and compliance patterns (SEP-0057 style) - Added
standards-reference.mdwith comprehensive SEP/CAP reference organized by protocol version, use case lookup tables, and implementation examples - Updated
contracts-soroban.mdto document Contract Constructors (CAP-0058, Protocol 22+) with deployment examples and comparison to initialize pattern - Updated
SKILL.mdto route to new documentation and expanded keywords for discoverability
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 16 comments.
| File | Description |
|---|---|
| skill/advanced-patterns.md | Comprehensive guide covering deployment patterns, upgradeable contracts, governance, DeFi primitives, optimization, and compliance - provides production-ready code examples for complex scenarios |
| skill/standards-reference.md | Complete SEP/CAP reference with protocol version mapping, quick lookup by use case, and CLI examples - fills gap in standards documentation |
| skill/contracts-soroban.md | Adds documentation for Contract Constructors (CAP-0058) including definition, deployment methods, rules, and comparison with initialize pattern |
| skill/SKILL.md | Updates routing and keywords to include new advanced patterns and standards documentation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ```rust | ||
| use soroban_sdk::{contract, contractimpl, Address, BytesN, Env}; | ||
|
|
||
| #[contractimpl] | ||
| impl MyContract { | ||
| /// Upgrade contract WASM. Only callable by admin. | ||
| pub fn upgrade(env: Env, new_wasm_hash: BytesN<32>) { | ||
| // Verify caller is admin | ||
| let admin: Address = env.storage().instance().get(&DataKey::Admin).unwrap(); | ||
| admin.require_auth(); | ||
|
|
||
| // Update the contract's WASM code | ||
| env.deployer().update_current_contract_wasm(new_wasm_hash); | ||
| } | ||
| } | ||
| ``` |
There was a problem hiding this comment.
The code example references DataKey::Admin but doesn't define the DataKey enum. This will cause a compilation error. The example should either include the DataKey definition or note that it's assumed to be defined elsewhere in the contract.
| Predict contract addresses before deployment using salt. | ||
|
|
||
| ```rust | ||
| use soroban_sdk::{Address, BytesN, Env, xdr::Hash}; |
There was a problem hiding this comment.
The unused import xdr::Hash should be removed. The function predict_address doesn't use the Hash type, making this import unnecessary.
| use soroban_sdk::{Address, BytesN, Env, xdr::Hash}; | |
| use soroban_sdk::{Address, BytesN, Env}; |
| ### Oracle Integration Pattern | ||
| Consume price feeds from oracle contracts. | ||
|
|
||
| ```rust | ||
| use soroban_sdk::{contract, contractimpl, contracttype, Address, Env, Symbol}; | ||
|
|
||
| #[contracttype] | ||
| #[derive(Clone)] | ||
| pub struct PriceData { | ||
| pub price: i128, // price with decimals | ||
| pub timestamp: u64, // ledger timestamp | ||
| pub decimals: u32, | ||
| } | ||
|
|
||
| // Oracle interface | ||
| mod oracle { | ||
| use soroban_sdk::{contractclient, Address, Env, Symbol}; | ||
| use super::PriceData; | ||
|
|
||
| #[contractclient(name = "OracleClient")] | ||
| pub trait OracleInterface { | ||
| fn get_price(env: Env, asset: Symbol) -> PriceData; | ||
| fn get_prices(env: Env, assets: soroban_sdk::Vec<Symbol>) -> soroban_sdk::Vec<PriceData>; | ||
| } | ||
| } | ||
|
|
||
| #[contractimpl] | ||
| impl MyDeFiContract { | ||
| pub fn get_collateral_value(env: Env, oracle_addr: Address, asset: Symbol, amount: i128) -> i128 { | ||
| let oracle = oracle::OracleClient::new(&env, &oracle_addr); | ||
| let price_data = oracle.get_price(&asset); | ||
|
|
||
| // Check price is fresh (within last ~10 minutes) | ||
| let max_age: u64 = 120; // ledgers | ||
| let current_ledger = env.ledger().sequence(); | ||
| if current_ledger - price_data.timestamp > max_age { | ||
| panic!("stale price"); | ||
| } | ||
|
|
||
| // Calculate value: amount * price / 10^decimals | ||
| let decimals_factor = 10i128.pow(price_data.decimals); | ||
| (amount * price_data.price) / decimals_factor | ||
| } | ||
| } | ||
| ``` |
There was a problem hiding this comment.
This code example is missing the #[contract] struct definition at the beginning. Without defining which struct the implementation is for, this code will not compile. Add a contract struct definition before the impl block.
| ```rust | ||
| #![no_std] | ||
| use soroban_sdk::{contract, contractimpl, Address, Env}; | ||
|
|
||
| #[contract] | ||
| pub struct MyContract; | ||
|
|
||
| #[contractimpl] | ||
| impl MyContract { | ||
| /// Constructor - called automatically when contract is deployed. | ||
| /// Must return () (void). Cannot be called after deployment. | ||
| pub fn __constructor(env: Env, admin: Address, initial_value: u32) { | ||
| env.storage().instance().set(&DataKey::Admin, &admin); | ||
| env.storage().instance().set(&DataKey::Value, &initial_value); | ||
| } | ||
|
|
||
| pub fn get_value(env: Env) -> u32 { | ||
| env.storage().instance().get(&DataKey::Value).unwrap() | ||
| } | ||
| } | ||
| ``` |
There was a problem hiding this comment.
This code references DataKey::Admin but doesn't define the DataKey enum. Since this is a constructor example showing a specific feature, add a comment indicating that DataKey should be defined elsewhere in the contract, or include a minimal DataKey definition for completeness.
|
|
||
| | SEP | Title | Description | Use When | | ||
| |-----|-------|-------------|----------| | ||
| | [SEP-0041](https://github.com/stellar/stellar-protocol/blob/master/ecosystem/sep-0041.md) | Soroban Token Interface | Standard token interface (balance, transfer, approve, allowance) | Building fungible tokens on Soroban | |
There was a problem hiding this comment.
There's an inconsistency in SEP-0041 link format. In standards-reference.md, SEP-0041 links to the GitHub protocol repository, which is consistent with all other SEP links. Consider updating any references to use the consistent GitHub link format.
| ## DeFi Patterns | ||
|
|
||
| ### Vault Pattern (SEP-0056 style) | ||
| Tokenized vault for yield-bearing deposits. | ||
|
|
||
| ```rust | ||
| use soroban_sdk::{contract, contractimpl, contracttype, token, Address, Env}; | ||
|
|
||
| #[contracttype] | ||
| pub enum DataKey { | ||
| Asset, // underlying asset address | ||
| TotalShares, | ||
| TotalAssets, | ||
| Shares(Address), // user shares | ||
| } | ||
|
|
||
| #[contractimpl] | ||
| impl Vault { | ||
| pub fn deposit(env: Env, user: Address, assets: i128) -> i128 { | ||
| user.require_auth(); | ||
|
|
||
| let asset_addr: Address = env.storage().instance().get(&DataKey::Asset).unwrap(); | ||
| let token = token::Client::new(&env, &asset_addr); | ||
|
|
||
| // Transfer assets to vault | ||
| token.transfer(&user, &env.current_contract_address(), &assets); | ||
|
|
||
| // Calculate shares to mint | ||
| let shares = Self::convert_to_shares(&env, assets); | ||
|
|
||
| // Update state | ||
| let mut total_shares: i128 = env.storage().instance().get(&DataKey::TotalShares).unwrap_or(0); | ||
| let mut total_assets: i128 = env.storage().instance().get(&DataKey::TotalAssets).unwrap_or(0); | ||
| let mut user_shares: i128 = env.storage().persistent().get(&DataKey::Shares(user.clone())).unwrap_or(0); | ||
|
|
||
| total_shares += shares; | ||
| total_assets += assets; | ||
| user_shares += shares; | ||
|
|
||
| env.storage().instance().set(&DataKey::TotalShares, &total_shares); | ||
| env.storage().instance().set(&DataKey::TotalAssets, &total_assets); | ||
| env.storage().persistent().set(&DataKey::Shares(user), &user_shares); | ||
|
|
||
| shares | ||
| } | ||
|
|
||
| pub fn withdraw(env: Env, user: Address, shares: i128) -> i128 { | ||
| user.require_auth(); | ||
|
|
||
| let user_shares: i128 = env.storage().persistent().get(&DataKey::Shares(user.clone())).unwrap_or(0); | ||
| if user_shares < shares { | ||
| panic!("insufficient shares"); | ||
| } | ||
|
|
||
| // Calculate assets to return | ||
| let assets = Self::convert_to_assets(&env, shares); | ||
|
|
||
| // Update state | ||
| let mut total_shares: i128 = env.storage().instance().get(&DataKey::TotalShares).unwrap(); | ||
| let mut total_assets: i128 = env.storage().instance().get(&DataKey::TotalAssets).unwrap(); | ||
|
|
||
| total_shares -= shares; | ||
| total_assets -= assets; | ||
|
|
||
| env.storage().instance().set(&DataKey::TotalShares, &total_shares); | ||
| env.storage().instance().set(&DataKey::TotalAssets, &total_assets); | ||
| env.storage().persistent().set(&DataKey::Shares(user.clone()), &(user_shares - shares)); | ||
|
|
||
| // Transfer assets to user | ||
| let asset_addr: Address = env.storage().instance().get(&DataKey::Asset).unwrap(); | ||
| let token = token::Client::new(&env, &asset_addr); | ||
| token.transfer(&env.current_contract_address(), &user, &assets); | ||
|
|
||
| assets | ||
| } | ||
|
|
||
| fn convert_to_shares(env: &Env, assets: i128) -> i128 { | ||
| let total_shares: i128 = env.storage().instance().get(&DataKey::TotalShares).unwrap_or(0); | ||
| let total_assets: i128 = env.storage().instance().get(&DataKey::TotalAssets).unwrap_or(0); | ||
|
|
||
| if total_shares == 0 || total_assets == 0 { | ||
| assets // 1:1 for first deposit | ||
| } else { | ||
| (assets * total_shares) / total_assets | ||
| } | ||
| } | ||
|
|
||
| fn convert_to_assets(env: &Env, shares: i128) -> i128 { | ||
| let total_shares: i128 = env.storage().instance().get(&DataKey::TotalShares).unwrap_or(0); | ||
| let total_assets: i128 = env.storage().instance().get(&DataKey::TotalAssets).unwrap_or(0); | ||
|
|
||
| if total_shares == 0 { | ||
| 0 | ||
| } else { | ||
| (shares * total_assets) / total_shares | ||
| } | ||
| } | ||
| } | ||
| ``` |
There was a problem hiding this comment.
This code example is missing the #[contract] struct definition at the beginning. Without defining which struct the implementation is for, this code will not compile. Add a contract struct definition before the impl block.
| #[contractimpl] | ||
| impl BatchContract { | ||
| /// Batch multiple transfers in one call | ||
| pub fn batch_transfer(env: Env, from: Address, transfers: Vec<Transfer>) { | ||
| from.require_auth(); | ||
|
|
||
| let token: Address = env.storage().instance().get(&DataKey::Token).unwrap(); | ||
| let token_client = token::Client::new(&env, &token); | ||
|
|
||
| for transfer in transfers.iter() { | ||
| token_client.transfer(&from, &transfer.to, &transfer.amount); | ||
| } | ||
| } | ||
| } | ||
| ``` |
There was a problem hiding this comment.
The code example references DataKey::Token but doesn't define the DataKey enum. This will cause a compilation error. The example should either include the DataKey definition at the beginning or add a comment noting that DataKey should be defined elsewhere.
| #[contractimpl] | ||
| impl LazyContract { | ||
| pub fn process_if_needed(env: Env, user: Address) { | ||
| // Check flag first (cheap read) | ||
| let needs_processing: bool = env | ||
| .storage() | ||
| .temporary() | ||
| .get(&DataKey::NeedsProcess(user.clone())) | ||
| .unwrap_or(false); | ||
|
|
||
| if !needs_processing { | ||
| return; // Early exit, no expensive reads | ||
| } | ||
|
|
||
| // Only now load full user data (expensive) | ||
| let user_data: UserData = env | ||
| .storage() | ||
| .persistent() | ||
| .get(&DataKey::UserData(user.clone())) | ||
| .unwrap(); | ||
|
|
||
| // Process... | ||
| } | ||
| } | ||
| ``` |
There was a problem hiding this comment.
This code example references DataKey::NeedsProcess and DataKey::UserData which are not defined, and UserData type is also undefined. The example needs to either define these types or add a comment indicating they should be defined elsewhere in the contract.
| ```rust | ||
| use soroban_sdk::{contract, contractimpl, contracttype, Address, BytesN, Env}; | ||
|
|
||
| #[contracttype] | ||
| #[derive(Clone)] | ||
| pub struct PendingUpgrade { | ||
| pub wasm_hash: BytesN<32>, | ||
| pub execute_after: u64, // ledger sequence | ||
| pub proposer: Address, | ||
| } | ||
|
|
||
| #[contracttype] | ||
| pub enum DataKey { | ||
| Admin, | ||
| PendingUpgrade, | ||
| TimelockDelay, // in ledgers (~5 sec each) | ||
| } | ||
|
|
||
| const DEFAULT_DELAY: u64 = 17280; // ~1 day | ||
|
|
||
| #[contractimpl] | ||
| impl TimelockContract { | ||
| /// Propose an upgrade (starts timelock) | ||
| pub fn propose_upgrade(env: Env, wasm_hash: BytesN<32>) { | ||
| let admin: Address = env.storage().instance().get(&DataKey::Admin).unwrap(); | ||
| admin.require_auth(); | ||
|
|
||
| let delay: u64 = env | ||
| .storage() | ||
| .instance() | ||
| .get(&DataKey::TimelockDelay) | ||
| .unwrap_or(DEFAULT_DELAY); | ||
|
|
||
| let pending = PendingUpgrade { | ||
| wasm_hash, | ||
| execute_after: env.ledger().sequence() + delay, | ||
| proposer: admin, | ||
| }; | ||
|
|
||
| env.storage().instance().set(&DataKey::PendingUpgrade, &pending); | ||
| } | ||
|
|
||
| /// Execute upgrade after timelock expires | ||
| pub fn execute_upgrade(env: Env) { | ||
| let pending: PendingUpgrade = env | ||
| .storage() | ||
| .instance() | ||
| .get(&DataKey::PendingUpgrade) | ||
| .expect("no pending upgrade"); | ||
|
|
||
| if env.ledger().sequence() < pending.execute_after { | ||
| panic!("timelock not expired"); | ||
| } | ||
|
|
||
| env.deployer().update_current_contract_wasm(pending.wasm_hash); | ||
| env.storage().instance().remove(&DataKey::PendingUpgrade); | ||
| } | ||
|
|
||
| /// Cancel pending upgrade | ||
| pub fn cancel_upgrade(env: Env) { | ||
| let admin: Address = env.storage().instance().get(&DataKey::Admin).unwrap(); | ||
| admin.require_auth(); | ||
|
|
||
| env.storage().instance().remove(&DataKey::PendingUpgrade); | ||
| } | ||
| } | ||
| ``` |
There was a problem hiding this comment.
This code example is missing the #[contract] struct definition at the beginning. Without defining which struct the implementation is for, this code will not compile. Add a contract struct definition before the impl block.
| **JavaScript SDK:** | ||
| ```typescript | ||
| import { Contract, Networks, TransactionBuilder } from "@stellar/stellar-sdk"; | ||
|
|
||
| // Constructor args are passed in the deploy transaction | ||
| const deployTx = await contract.deploy({ | ||
| wasmHash: wasmHash, | ||
| constructorArgs: [ | ||
| nativeToScVal(adminAddress, { type: "address" }), | ||
| nativeToScVal(100, { type: "u32" }), | ||
| ], | ||
| }); | ||
| ``` |
There was a problem hiding this comment.
The TypeScript code example is incomplete and may not reflect the actual Stellar SDK API. The variables contract, wasmHash, adminAddress, and nativeToScVal are referenced but not defined or imported. Consider providing a more complete example with proper imports and variable definitions, or clearly indicate which parts are pseudocode.
- Add quick routing links for advanced patterns and SEP/CAP reference - Update progressive disclosure section - Expand keywords with new topics (constructor, upgradeable, factory, governance, multisig, vault, oracle, compliance, specific SEP/CAP numbers)
b0d3dc6 to
db7a020
Compare
- Remove non-existent deploy_v2() and deployed_address() methods - Replace factory pattern with CLI deployment and cross-contract client pattern - Fix oracle integration to use contractimport! correctly - Update JS SDK example to use Operation.createContractV2 Based on patterns from Neko-DApp contracts.
- Remove env.invoke_contract() usage, not in official examples - Use contractimport! + Client::new() pattern throughout - Fix Upgrader pattern to use typed client calls - Add Factory Pattern with deploy_v2 from official deployer example - Fix Oracle to use env.ledger().timestamp() for time checks - Fix ClawbackEvent to use struct syntax with #[contractevent] - Add missing DataKey enum to constructor example - Add missing imports to batching example - Simplify Cross-Contract Calls section to official patterns only
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 12 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ### Multi-Signature Pattern | ||
| Require multiple approvals for sensitive operations. | ||
|
|
||
| ```rust | ||
| use soroban_sdk::{contract, contractimpl, contracttype, Address, BytesN, Env, Map, Vec}; | ||
|
|
||
| #[contracttype] | ||
| #[derive(Clone)] | ||
| pub struct Proposal { | ||
| pub id: u64, | ||
| pub action: ProposalAction, | ||
| pub approvals: Vec<Address>, | ||
| pub executed: bool, | ||
| } | ||
|
|
||
| #[contracttype] | ||
| #[derive(Clone)] | ||
| pub enum ProposalAction { | ||
| Upgrade(BytesN<32>), | ||
| Transfer { to: Address, amount: i128 }, | ||
| AddSigner(Address), | ||
| RemoveSigner(Address), | ||
| ChangeThreshold(u32), | ||
| } | ||
|
|
||
| #[contracttype] | ||
| pub enum DataKey { | ||
| Signers, | ||
| Threshold, | ||
| ProposalCount, | ||
| Proposal(u64), | ||
| } | ||
|
|
||
| #[contractimpl] | ||
| impl MultisigContract { | ||
| pub fn initialize(env: Env, signers: Vec<Address>, threshold: u32) { | ||
| if env.storage().instance().has(&DataKey::Signers) { | ||
| panic!("already initialized"); | ||
| } | ||
| if threshold as usize > signers.len() || threshold == 0 { | ||
| panic!("invalid threshold"); | ||
| } | ||
|
|
||
| env.storage().instance().set(&DataKey::Signers, &signers); | ||
| env.storage().instance().set(&DataKey::Threshold, &threshold); | ||
| env.storage().instance().set(&DataKey::ProposalCount, &0u64); | ||
| } | ||
|
|
||
| pub fn propose(env: Env, proposer: Address, action: ProposalAction) -> u64 { | ||
| proposer.require_auth(); | ||
| Self::require_signer(&env, &proposer); | ||
|
|
||
| let mut count: u64 = env.storage().instance().get(&DataKey::ProposalCount).unwrap(); | ||
| count += 1; | ||
|
|
||
| let proposal = Proposal { | ||
| id: count, | ||
| action, | ||
| approvals: Vec::new(&env), | ||
| executed: false, | ||
| }; | ||
|
|
||
| env.storage().persistent().set(&DataKey::Proposal(count), &proposal); | ||
| env.storage().instance().set(&DataKey::ProposalCount, &count); | ||
|
|
||
| count | ||
| } | ||
|
|
||
| pub fn approve(env: Env, signer: Address, proposal_id: u64) { | ||
| signer.require_auth(); | ||
| Self::require_signer(&env, &signer); | ||
|
|
||
| let mut proposal: Proposal = env | ||
| .storage() | ||
| .persistent() | ||
| .get(&DataKey::Proposal(proposal_id)) | ||
| .expect("proposal not found"); | ||
|
|
||
| if proposal.executed { | ||
| panic!("already executed"); | ||
| } | ||
|
|
||
| // Check not already approved by this signer | ||
| for approved in proposal.approvals.iter() { | ||
| if approved == signer { | ||
| panic!("already approved"); | ||
| } | ||
| } | ||
|
|
||
| proposal.approvals.push_back(signer); | ||
| env.storage().persistent().set(&DataKey::Proposal(proposal_id), &proposal); | ||
| } | ||
|
|
||
| pub fn execute(env: Env, proposal_id: u64) { | ||
| let mut proposal: Proposal = env | ||
| .storage() | ||
| .persistent() | ||
| .get(&DataKey::Proposal(proposal_id)) | ||
| .expect("proposal not found"); | ||
|
|
||
| if proposal.executed { | ||
| panic!("already executed"); | ||
| } | ||
|
|
||
| let threshold: u32 = env.storage().instance().get(&DataKey::Threshold).unwrap(); | ||
| if (proposal.approvals.len() as u32) < threshold { | ||
| panic!("not enough approvals"); | ||
| } | ||
|
|
||
| proposal.executed = true; | ||
| env.storage().persistent().set(&DataKey::Proposal(proposal_id), &proposal); | ||
|
|
||
| // Execute the action | ||
| match proposal.action { | ||
| ProposalAction::Upgrade(wasm_hash) => { | ||
| env.deployer().update_current_contract_wasm(wasm_hash); | ||
| } | ||
| ProposalAction::ChangeThreshold(new_threshold) => { | ||
| env.storage().instance().set(&DataKey::Threshold, &new_threshold); | ||
| } | ||
| // Handle other actions... | ||
| _ => {} | ||
| } | ||
| } | ||
|
|
||
| fn require_signer(env: &Env, addr: &Address) { | ||
| let signers: Vec<Address> = env.storage().instance().get(&DataKey::Signers).unwrap(); | ||
| let mut is_signer = false; | ||
| for s in signers.iter() { | ||
| if &s == addr { | ||
| is_signer = true; | ||
| break; | ||
| } | ||
| } | ||
| if !is_signer { | ||
| panic!("not a signer"); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The MultisigContract code is missing the contract struct declaration. Add the following before line 348:
#[contract]
pub struct MultisigContract;Without this declaration, the code example won't compile.
| ### Storage Optimization | ||
| ```rust | ||
| // Use compact data structures | ||
| #[contracttype] | ||
| pub enum DataKey { | ||
| // Use u32 IDs instead of full addresses when possible | ||
| UserById(u32), | ||
| IdByUser(Address), | ||
| // Pack related data together | ||
| UserData(u32), // contains balance, status, timestamp in one struct | ||
| } | ||
|
|
||
| // Use appropriate storage type | ||
| impl StorageOptimized { | ||
| pub fn set_data(env: Env, key: DataKey, value: SomeData) { | ||
| // Temporary: cheap, auto-deleted (use for caches, flags) | ||
| env.storage().temporary().set(&DataKey::TempFlag, &true); | ||
|
|
||
| // Instance: shared across contract (use for global config) | ||
| env.storage().instance().set(&DataKey::Config, &config); | ||
|
|
||
| // Persistent: per-key TTL (use for user balances, important state) | ||
| env.storage().persistent().set(&DataKey::Balance(user), &balance); | ||
| } | ||
| } |
There was a problem hiding this comment.
The StorageOptimized pattern code is incomplete. The impl StorageOptimized block is missing the contract struct declaration and necessary imports. Add the following before line 751:
use soroban_sdk::{contract, contractimpl, contracttype, Env};
#[contract]
pub struct StorageOptimized;Additionally, the SomeData and Config types need to be defined or replaced with concrete types.
| pub fn deposit(env: Env, user: Address, assets: i128) -> i128 { | ||
| user.require_auth(); | ||
|
|
||
| let asset_addr: Address = env.storage().instance().get(&DataKey::Asset).unwrap(); | ||
| let token = token::Client::new(&env, &asset_addr); | ||
|
|
||
| // Transfer assets to vault | ||
| token.transfer(&user, &env.current_contract_address(), &assets); | ||
|
|
||
| // Calculate shares to mint | ||
| let shares = Self::convert_to_shares(&env, assets); | ||
|
|
||
| // Update state | ||
| let mut total_shares: i128 = env.storage().instance().get(&DataKey::TotalShares).unwrap_or(0); | ||
| let mut total_assets: i128 = env.storage().instance().get(&DataKey::TotalAssets).unwrap_or(0); | ||
| let mut user_shares: i128 = env.storage().persistent().get(&DataKey::Shares(user.clone())).unwrap_or(0); | ||
|
|
||
| total_shares += shares; | ||
| total_assets += assets; | ||
| user_shares += shares; | ||
|
|
||
| env.storage().instance().set(&DataKey::TotalShares, &total_shares); | ||
| env.storage().instance().set(&DataKey::TotalAssets, &total_assets); | ||
| env.storage().persistent().set(&DataKey::Shares(user), &user_shares); | ||
|
|
||
| shares | ||
| } |
There was a problem hiding this comment.
In the deposit function, state is updated after the token transfer. If the transfer fails, the function will panic and revert, so this is safe. However, as a best practice for clarity and to follow checks-effects-interactions pattern (even though Soroban prevents reentrancy), consider moving the state updates before the transfer. This makes the code more defensive and easier to audit.
The same applies to the withdraw function where the transfer happens after state updates - this ordering is actually correct and follows best practices.
| ### Liquidity Pool (Constant Product AMM) | ||
| ```rust | ||
| use soroban_sdk::{contract, contractimpl, contracttype, token, Address, Env}; | ||
|
|
||
| #[contracttype] | ||
| pub enum DataKey { | ||
| TokenA, | ||
| TokenB, | ||
| ReserveA, | ||
| ReserveB, | ||
| TotalLiquidity, | ||
| Liquidity(Address), | ||
| } | ||
|
|
||
| const FEE_BPS: i128 = 30; // 0.3% fee | ||
|
|
||
| #[contractimpl] | ||
| impl LiquidityPool { | ||
| /// Swap token A for token B | ||
| pub fn swap_a_for_b(env: Env, user: Address, amount_in: i128, min_out: i128) -> i128 { | ||
| user.require_auth(); | ||
|
|
||
| let reserve_a: i128 = env.storage().instance().get(&DataKey::ReserveA).unwrap(); | ||
| let reserve_b: i128 = env.storage().instance().get(&DataKey::ReserveB).unwrap(); | ||
|
|
||
| // Calculate output with fee: (amount_in * (10000 - fee) * reserve_b) / (reserve_a * 10000 + amount_in * (10000 - fee)) | ||
| let amount_in_with_fee = amount_in * (10000 - FEE_BPS); | ||
| let numerator = amount_in_with_fee * reserve_b; | ||
| let denominator = reserve_a * 10000 + amount_in_with_fee; | ||
| let amount_out = numerator / denominator; | ||
|
|
||
| if amount_out < min_out { | ||
| panic!("slippage exceeded"); | ||
| } | ||
|
|
||
| // Transfer tokens | ||
| let token_a: Address = env.storage().instance().get(&DataKey::TokenA).unwrap(); | ||
| let token_b: Address = env.storage().instance().get(&DataKey::TokenB).unwrap(); | ||
|
|
||
| token::Client::new(&env, &token_a).transfer(&user, &env.current_contract_address(), &amount_in); | ||
| token::Client::new(&env, &token_b).transfer(&env.current_contract_address(), &user, &amount_out); | ||
|
|
||
| // Update reserves | ||
| env.storage().instance().set(&DataKey::ReserveA, &(reserve_a + amount_in)); | ||
| env.storage().instance().set(&DataKey::ReserveB, &(reserve_b - amount_out)); | ||
|
|
||
| amount_out | ||
| } | ||
|
|
||
| /// Add liquidity | ||
| pub fn add_liquidity(env: Env, user: Address, amount_a: i128, amount_b: i128) -> i128 { | ||
| user.require_auth(); | ||
|
|
||
| let reserve_a: i128 = env.storage().instance().get(&DataKey::ReserveA).unwrap_or(0); | ||
| let reserve_b: i128 = env.storage().instance().get(&DataKey::ReserveB).unwrap_or(0); | ||
| let total_liquidity: i128 = env.storage().instance().get(&DataKey::TotalLiquidity).unwrap_or(0); | ||
|
|
||
| let liquidity: i128; | ||
| if total_liquidity == 0 { | ||
| // First deposit - liquidity = sqrt(amount_a * amount_b) | ||
| liquidity = Self::sqrt(amount_a * amount_b); | ||
| } else { | ||
| // Proportional deposit | ||
| let liquidity_a = (amount_a * total_liquidity) / reserve_a; | ||
| let liquidity_b = (amount_b * total_liquidity) / reserve_b; | ||
| liquidity = if liquidity_a < liquidity_b { liquidity_a } else { liquidity_b }; | ||
| } | ||
|
|
||
| // Transfer tokens to pool | ||
| let token_a: Address = env.storage().instance().get(&DataKey::TokenA).unwrap(); | ||
| let token_b: Address = env.storage().instance().get(&DataKey::TokenB).unwrap(); | ||
|
|
||
| token::Client::new(&env, &token_a).transfer(&user, &env.current_contract_address(), &amount_a); | ||
| token::Client::new(&env, &token_b).transfer(&user, &env.current_contract_address(), &amount_b); | ||
|
|
||
| // Update state | ||
| let user_liquidity: i128 = env.storage().persistent().get(&DataKey::Liquidity(user.clone())).unwrap_or(0); | ||
| env.storage().persistent().set(&DataKey::Liquidity(user), &(user_liquidity + liquidity)); | ||
| env.storage().instance().set(&DataKey::TotalLiquidity, &(total_liquidity + liquidity)); | ||
| env.storage().instance().set(&DataKey::ReserveA, &(reserve_a + amount_a)); | ||
| env.storage().instance().set(&DataKey::ReserveB, &(reserve_b + amount_b)); | ||
|
|
||
| liquidity | ||
| } | ||
|
|
||
| fn sqrt(n: i128) -> i128 { | ||
| if n == 0 { return 0; } | ||
| let mut x = n; | ||
| let mut y = (x + 1) / 2; | ||
| while y < x { | ||
| x = y; | ||
| y = (x + n / x) / 2; | ||
| } | ||
| x | ||
| } | ||
| } |
There was a problem hiding this comment.
The LiquidityPool code is missing the contract struct declaration. Add the following before line 617:
#[contract]
pub struct LiquidityPool;Without this declaration, the code example won't compile.
| ### Lazy Loading Pattern | ||
| Only load data when needed. | ||
|
|
||
| ```rust | ||
| #[contractimpl] | ||
| impl LazyContract { | ||
| pub fn process_if_needed(env: Env, user: Address) { | ||
| // Check flag first (cheap read) | ||
| let needs_processing: bool = env | ||
| .storage() | ||
| .temporary() | ||
| .get(&DataKey::NeedsProcess(user.clone())) | ||
| .unwrap_or(false); | ||
|
|
||
| if !needs_processing { | ||
| return; // Early exit, no expensive reads | ||
| } | ||
|
|
||
| // Only now load full user data (expensive) | ||
| let user_data: UserData = env | ||
| .storage() | ||
| .persistent() | ||
| .get(&DataKey::UserData(user.clone())) | ||
| .unwrap(); | ||
|
|
||
| // Process... | ||
| } | ||
| } |
There was a problem hiding this comment.
The LazyContract code is missing the contract struct declaration and necessary imports. Add the following before line 769:
use soroban_sdk::{contract, contractimpl, contracttype, Address, Env};
#[contract]
pub struct LazyContract;Additionally, the DataKey and UserData types need to be defined for this example to be complete.
| | 20 | Soroban launch (smart contracts) | Feb 2024 | | ||
| | 21 | Secp256r1 (passkeys), TTL functions | Aug 2024 | | ||
| | 22 | Constructors, BLS12-381 | Nov 2024 | | ||
| | 23 | State optimization, unified events | Jan 2025 | |
There was a problem hiding this comment.
The Protocol Version Quick Reference table jumps from Protocol 23 to Protocol 25, skipping Protocol 24. If Protocol 24 was skipped or had no smart contract-related features, this should be documented for clarity. Otherwise, if Protocol 24 exists with relevant features, it should be included in the table. Consider adding a note if Protocol 24 was intentionally skipped in the Stellar release cycle.
| | 23 | State optimization, unified events | Jan 2025 | | |
| | 23 | State optimization, unified events | Jan 2025 | | |
| | 24 | No Soroban/smart contract–specific changes relevant to this guide | — | |
| - Upgradeable contract patterns (SEP-0049) | ||
| - Governance and multi-sig patterns | ||
| - DeFi primitives (vaults, oracles, liquidity pools) | ||
| - Gas/resource optimization strategies |
There was a problem hiding this comment.
The term "Gas/resource" is used here, mixing Ethereum terminology (Gas) with Stellar terminology (resource). While this is understandable for developer familiarity, for consistency with Stellar documentation, consider using "Resource optimization" or "Fee optimization" instead. The codebase uses "Gas/resource" in multiple places (contracts-soroban.md, advanced-patterns.md), so if changed here, it should be changed throughout for consistency.
| - Gas/resource optimization strategies | |
| - Resource optimization strategies |
| ### Vault Pattern (SEP-0056 style) | ||
| Tokenized vault for yield-bearing deposits. | ||
|
|
||
| ```rust | ||
| use soroban_sdk::{contract, contractimpl, contracttype, token, Address, Env}; | ||
|
|
||
| #[contracttype] | ||
| pub enum DataKey { | ||
| Asset, // underlying asset address | ||
| TotalShares, | ||
| TotalAssets, | ||
| Shares(Address), // user shares | ||
| } | ||
|
|
||
| #[contractimpl] | ||
| impl Vault { | ||
| pub fn deposit(env: Env, user: Address, assets: i128) -> i128 { | ||
| user.require_auth(); | ||
|
|
||
| let asset_addr: Address = env.storage().instance().get(&DataKey::Asset).unwrap(); | ||
| let token = token::Client::new(&env, &asset_addr); | ||
|
|
||
| // Transfer assets to vault | ||
| token.transfer(&user, &env.current_contract_address(), &assets); | ||
|
|
||
| // Calculate shares to mint | ||
| let shares = Self::convert_to_shares(&env, assets); | ||
|
|
||
| // Update state | ||
| let mut total_shares: i128 = env.storage().instance().get(&DataKey::TotalShares).unwrap_or(0); | ||
| let mut total_assets: i128 = env.storage().instance().get(&DataKey::TotalAssets).unwrap_or(0); | ||
| let mut user_shares: i128 = env.storage().persistent().get(&DataKey::Shares(user.clone())).unwrap_or(0); | ||
|
|
||
| total_shares += shares; | ||
| total_assets += assets; | ||
| user_shares += shares; | ||
|
|
||
| env.storage().instance().set(&DataKey::TotalShares, &total_shares); | ||
| env.storage().instance().set(&DataKey::TotalAssets, &total_assets); | ||
| env.storage().persistent().set(&DataKey::Shares(user), &user_shares); | ||
|
|
||
| shares | ||
| } | ||
|
|
||
| pub fn withdraw(env: Env, user: Address, shares: i128) -> i128 { | ||
| user.require_auth(); | ||
|
|
||
| let user_shares: i128 = env.storage().persistent().get(&DataKey::Shares(user.clone())).unwrap_or(0); | ||
| if user_shares < shares { | ||
| panic!("insufficient shares"); | ||
| } | ||
|
|
||
| // Calculate assets to return | ||
| let assets = Self::convert_to_assets(&env, shares); | ||
|
|
||
| // Update state | ||
| let mut total_shares: i128 = env.storage().instance().get(&DataKey::TotalShares).unwrap(); | ||
| let mut total_assets: i128 = env.storage().instance().get(&DataKey::TotalAssets).unwrap(); | ||
|
|
||
| total_shares -= shares; | ||
| total_assets -= assets; | ||
|
|
||
| env.storage().instance().set(&DataKey::TotalShares, &total_shares); | ||
| env.storage().instance().set(&DataKey::TotalAssets, &total_assets); | ||
| env.storage().persistent().set(&DataKey::Shares(user.clone()), &(user_shares - shares)); | ||
|
|
||
| // Transfer assets to user | ||
| let asset_addr: Address = env.storage().instance().get(&DataKey::Asset).unwrap(); | ||
| let token = token::Client::new(&env, &asset_addr); | ||
| token.transfer(&env.current_contract_address(), &user, &assets); | ||
|
|
||
| assets | ||
| } | ||
|
|
||
| fn convert_to_shares(env: &Env, assets: i128) -> i128 { | ||
| let total_shares: i128 = env.storage().instance().get(&DataKey::TotalShares).unwrap_or(0); | ||
| let total_assets: i128 = env.storage().instance().get(&DataKey::TotalAssets).unwrap_or(0); | ||
|
|
||
| if total_shares == 0 || total_assets == 0 { | ||
| assets // 1:1 for first deposit | ||
| } else { | ||
| (assets * total_shares) / total_assets | ||
| } | ||
| } | ||
|
|
||
| fn convert_to_assets(env: &Env, shares: i128) -> i128 { | ||
| let total_shares: i128 = env.storage().instance().get(&DataKey::TotalShares).unwrap_or(0); | ||
| let total_assets: i128 = env.storage().instance().get(&DataKey::TotalAssets).unwrap_or(0); | ||
|
|
||
| if total_shares == 0 { | ||
| 0 | ||
| } else { | ||
| (shares * total_assets) / total_shares | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The Vault pattern code is missing the contract struct declaration. Add the following before the #[contractimpl] block:
#[contract]
pub struct Vault;Without this declaration, the code example is incomplete and won't compile. This is necessary for the Soroban SDK to recognize this as a contract implementation.
| ### Time-locked Operations | ||
| Delay sensitive operations to allow review. | ||
|
|
||
| ```rust | ||
| use soroban_sdk::{contract, contractimpl, contracttype, Address, BytesN, Env}; | ||
|
|
||
| #[contracttype] | ||
| #[derive(Clone)] | ||
| pub struct PendingUpgrade { | ||
| pub wasm_hash: BytesN<32>, | ||
| pub execute_after: u64, // ledger sequence | ||
| pub proposer: Address, | ||
| } | ||
|
|
||
| #[contracttype] | ||
| pub enum DataKey { | ||
| Admin, | ||
| PendingUpgrade, | ||
| TimelockDelay, // in ledgers (~5 sec each) | ||
| } | ||
|
|
||
| const DEFAULT_DELAY: u64 = 17280; // ~1 day | ||
|
|
||
| #[contractimpl] | ||
| impl TimelockContract { | ||
| /// Propose an upgrade (starts timelock) | ||
| pub fn propose_upgrade(env: Env, wasm_hash: BytesN<32>) { | ||
| let admin: Address = env.storage().instance().get(&DataKey::Admin).unwrap(); | ||
| admin.require_auth(); | ||
|
|
||
| let delay: u64 = env | ||
| .storage() | ||
| .instance() | ||
| .get(&DataKey::TimelockDelay) | ||
| .unwrap_or(DEFAULT_DELAY); | ||
|
|
||
| let pending = PendingUpgrade { | ||
| wasm_hash, | ||
| execute_after: env.ledger().sequence() + delay, | ||
| proposer: admin, | ||
| }; | ||
|
|
||
| env.storage().instance().set(&DataKey::PendingUpgrade, &pending); | ||
| } | ||
|
|
||
| /// Execute upgrade after timelock expires | ||
| pub fn execute_upgrade(env: Env) { | ||
| let pending: PendingUpgrade = env | ||
| .storage() | ||
| .instance() | ||
| .get(&DataKey::PendingUpgrade) | ||
| .expect("no pending upgrade"); | ||
|
|
||
| if env.ledger().sequence() < pending.execute_after { | ||
| panic!("timelock not expired"); | ||
| } | ||
|
|
||
| env.deployer().update_current_contract_wasm(pending.wasm_hash); | ||
| env.storage().instance().remove(&DataKey::PendingUpgrade); | ||
| } | ||
|
|
||
| /// Cancel pending upgrade | ||
| pub fn cancel_upgrade(env: Env) { | ||
| let admin: Address = env.storage().instance().get(&DataKey::Admin).unwrap(); | ||
| admin.require_auth(); | ||
|
|
||
| env.storage().instance().remove(&DataKey::PendingUpgrade); | ||
| } | ||
| } |
There was a problem hiding this comment.
The TimelockContract code is missing the contract struct declaration. Add the following before line 267:
#[contract]
pub struct TimelockContract;Without this declaration, the code example won't compile as the Soroban SDK needs this to recognize it as a contract.
| pub fn initialize(env: Env, signers: Vec<Address>, threshold: u32) { | ||
| if env.storage().instance().has(&DataKey::Signers) { | ||
| panic!("already initialized"); | ||
| } | ||
| if threshold as usize > signers.len() || threshold == 0 { | ||
| panic!("invalid threshold"); | ||
| } | ||
|
|
||
| env.storage().instance().set(&DataKey::Signers, &signers); | ||
| env.storage().instance().set(&DataKey::Threshold, &threshold); | ||
| env.storage().instance().set(&DataKey::ProposalCount, &0u64); |
There was a problem hiding this comment.
The initialize function is publicly callable and does not perform any require_auth check to ensure that only an authorized party can set signers and threshold. If this contract is deployed and not atomically initialized, an attacker can front‑run or simply be the first caller of initialize, set themselves (or arbitrary addresses) as signers with a low threshold, and then control all privileged proposal actions such as Upgrade or Transfer. You should enforce authorization for initialization (for example by requiring a designated deployer or one of the intended signers to authorize the call, or by moving this logic into a constructor) so that only trusted accounts can ever configure the multisig parameters.
contracts-soroban.md: - Reorganize structure: Core Contract Structure before Constructors - Move deploy CLI commands to Building and Deploying section - Remove JavaScript code (belongs in frontend-stellar-sdk.md) advanced-patterns.md: - Add missing #[contract] attributes to all examples - Remove duplicate CLI deploy commands (already in contracts-soroban.md) - Keep only advanced patterns: Factory, Upgrades, Governance, DeFi
…thub.com/aguilar1x/stellar-dev-skill into feat/add-advanced-patterns-and-standards
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 8 comments.
Comments suppressed due to low confidence (1)
skill/SKILL.md:142
- The keywords list is duplicated: lines 139-142 repeat entries already included above (e.g., freighter/stellar-sdk/soroban-sdk...). This makes the section harder to maintain and can skew keyword-based discoverability; please dedupe into a single comma-separated list.
cap-0046, cap-0051, cap-0058, cap-0059, cap-0074, cap-0075
freighter, stellar-sdk, soroban-sdk, stellar-cli, trustline, anchor, sep, passkey,
smart wallet, sac, stellar asset contract, defi, token, nft, scaffold stellar,
zero-knowledge, zk, zk-snark, groth16, bn254, poseidon, pairing, privacy, confidential,
x-ray, protocol 25, noir, risc zero, privacy pool, merkle tree
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Admin | ||
| fn initialize(env: Env, admin: Address, decimal: u32, name: String, symbol: String); | ||
|
|
||
| // Getters | ||
| fn name(env: Env) -> String; | ||
| fn symbol(env: Env) -> Symbol; | ||
| fn decimals(env: Env) -> u32; |
There was a problem hiding this comment.
In the SEP-0041 trait example, initialize takes symbol: String but the symbol() getter returns Symbol. This is internally inconsistent and likely to confuse implementers; align the types (and adjust the imports) so the setter/getter use the same representation.
| UserById(u32), | ||
| IdByUser(Address), | ||
| // Pack related data together | ||
| UserData(u32), // contains balance, status, timestamp in one struct |
There was a problem hiding this comment.
In the “Storage Optimization” snippet, the DataKey enum defined here doesn’t include the variants used later in the same snippet (TempFlag, Config, Balance). Either add those variants to the enum in this example or rename the later uses to match the defined variants.
| UserData(u32), // contains balance, status, timestamp in one struct | |
| UserData(u32), // contains balance, status, timestamp in one struct | |
| // Additional keys for various storage tiers | |
| TempFlag, | |
| Config, | |
| Balance(u32), |
| pub fn process_if_needed(env: Env, user: Address) { | ||
| // Check flag first (cheap read) | ||
| let needs_processing: bool = env | ||
| .storage() | ||
| .temporary() | ||
| .get(&DataKey::NeedsProcess(user.clone())) | ||
| .unwrap_or(false); |
There was a problem hiding this comment.
The lazy-loading example reads DataKey::NeedsProcess(...) but the DataKey enum (or that variant) isn’t defined in this section. Please add the missing DataKey definition/variant (or reference it) so the example is complete.
| // Only now load full user data (expensive) | ||
| let user_data: UserData = env | ||
| .storage() | ||
| .persistent() | ||
| .get(&DataKey::UserData(user.clone())) | ||
| .unwrap(); |
There was a problem hiding this comment.
This snippet uses UserData and DataKey::UserData(...) without defining UserData (and the DataKey variant in this context). Please add the missing struct/enum pieces or clarify that they’re placeholders, otherwise the example can’t be followed/copied reliably.
|
|
||
| let reserve_a: i128 = env.storage().instance().get(&DataKey::ReserveA).unwrap(); | ||
| let reserve_b: i128 = env.storage().instance().get(&DataKey::ReserveB).unwrap(); | ||
|
|
||
| // Calculate output with fee: (amount_in * (10000 - fee) * reserve_b) / (reserve_a * 10000 + amount_in * (10000 - fee)) | ||
| let amount_in_with_fee = amount_in * (10000 - FEE_BPS); | ||
| let numerator = amount_in_with_fee * reserve_b; | ||
| let denominator = reserve_a * 10000 + amount_in_with_fee; | ||
| let amount_out = numerator / denominator; | ||
|
|
||
| if amount_out < min_out { | ||
| panic!("slippage exceeded"); | ||
| } |
There was a problem hiding this comment.
The AMM swap example doesn’t validate inputs/reserves (e.g., amount_in > 0, reserve_a > 0, reserve_b > 0). As written, denominator can be 0 (division by zero) and negative/zero amounts can produce incorrect reserve updates; add explicit checks (or at least note them) to make this pattern safe to apply.
| let reserve_a: i128 = env.storage().instance().get(&DataKey::ReserveA).unwrap(); | |
| let reserve_b: i128 = env.storage().instance().get(&DataKey::ReserveB).unwrap(); | |
| // Calculate output with fee: (amount_in * (10000 - fee) * reserve_b) / (reserve_a * 10000 + amount_in * (10000 - fee)) | |
| let amount_in_with_fee = amount_in * (10000 - FEE_BPS); | |
| let numerator = amount_in_with_fee * reserve_b; | |
| let denominator = reserve_a * 10000 + amount_in_with_fee; | |
| let amount_out = numerator / denominator; | |
| if amount_out < min_out { | |
| panic!("slippage exceeded"); | |
| } | |
| if amount_in <= 0 { | |
| panic!("amount_in must be positive"); | |
| } | |
| if min_out <= 0 { | |
| panic!("min_out must be positive"); | |
| } | |
| let reserve_a: i128 = env.storage().instance().get(&DataKey::ReserveA).unwrap(); | |
| let reserve_b: i128 = env.storage().instance().get(&DataKey::ReserveB).unwrap(); | |
| if reserve_a <= 0 || reserve_b <= 0 { | |
| panic!("pool reserves must be positive"); | |
| } | |
| // Calculate output with fee: (amount_in * (10000 - fee) * reserve_b) / (reserve_a * 10000 + amount_in * (10000 - fee)) | |
| let amount_in_with_fee = amount_in * (10000 - FEE_BPS); | |
| if amount_in_with_fee <= 0 { | |
| panic!("amount_in with fee must be positive"); | |
| } | |
| let numerator = amount_in_with_fee * reserve_b; | |
| let denominator = reserve_a * 10000 + amount_in_with_fee; | |
| if denominator <= 0 { | |
| panic!("invalid swap: denominator must be positive"); | |
| } | |
| let amount_out = numerator / denominator; | |
| if amount_out <= 0 { | |
| panic!("amount_out must be positive"); | |
| } | |
| if amount_out < min_out { | |
| panic!("slippage exceeded"); | |
| } | |
| if amount_out > reserve_b { | |
| panic!("insufficient liquidity"); | |
| } |
| use soroban_sdk::{contractevent, Address, Env}; | ||
|
|
||
| #[contractevent] | ||
| pub struct ClawbackEvent { | ||
| pub from: Address, | ||
| pub amount: i128, | ||
| } | ||
|
|
||
| #[contract] | ||
| pub struct ClawbackToken; | ||
|
|
||
| #[contractimpl] | ||
| impl ClawbackToken { | ||
| /// Clawback tokens from an address (compliance/legal requirement) | ||
| pub fn clawback(env: Env, from: Address, amount: i128) { | ||
| // Only clawback admin can call | ||
| let clawback_admin: Address = env | ||
| .storage() | ||
| .instance() | ||
| .get(&DataKey::ClawbackAdmin) | ||
| .unwrap(); | ||
| clawback_admin.require_auth(); |
There was a problem hiding this comment.
The clawback example isn’t self-contained: it uses #[contract]/#[contractimpl] without importing those macros, and it references DataKey::ClawbackAdmin / DataKey::Balance(...) without defining DataKey. Please add the missing imports + DataKey definition (or clearly mark the missing pieces as placeholders).
| /// Upgrade contract WASM. Only callable by admin. | ||
| pub fn upgrade(env: Env, new_wasm_hash: BytesN<32>) { | ||
| // Verify caller is admin | ||
| let admin: Address = env.storage().instance().get(&DataKey::Admin).unwrap(); | ||
| admin.require_auth(); |
There was a problem hiding this comment.
The “Basic Upgrade Pattern” snippet uses DataKey::Admin but DataKey isn’t defined in this snippet/section. Please either include the DataKey definition here or reference a previously-defined one so readers can copy/paste the example without missing types.
| /// Add liquidity | ||
| pub fn add_liquidity(env: Env, user: Address, amount_a: i128, amount_b: i128) -> i128 { | ||
| user.require_auth(); | ||
|
|
||
| let reserve_a: i128 = env.storage().instance().get(&DataKey::ReserveA).unwrap_or(0); | ||
| let reserve_b: i128 = env.storage().instance().get(&DataKey::ReserveB).unwrap_or(0); | ||
| let total_liquidity: i128 = env.storage().instance().get(&DataKey::TotalLiquidity).unwrap_or(0); | ||
|
|
||
| let liquidity: i128; | ||
| if total_liquidity == 0 { | ||
| // First deposit - liquidity = sqrt(amount_a * amount_b) | ||
| liquidity = Self::sqrt(amount_a * amount_b); | ||
| } else { | ||
| // Proportional deposit | ||
| let liquidity_a = (amount_a * total_liquidity) / reserve_a; | ||
| let liquidity_b = (amount_b * total_liquidity) / reserve_b; | ||
| liquidity = if liquidity_a < liquidity_b { liquidity_a } else { liquidity_b }; | ||
| } |
There was a problem hiding this comment.
In add_liquidity, when total_liquidity != 0 the code divides by reserve_a/reserve_b without guarding against 0 reserves, and it allows negative/zero deposits (which also makes sqrt(amount_a * amount_b) undefined for negatives). Add input validation and reserve sanity checks (or document required preconditions) so readers don’t copy a footgun.
Refine Stellar dev skill docs and integrate PR #3 content
This PR significantly expands the skill coverage with advanced Soroban patterns and comprehensive SEP/CAP documentation that was missing.
New Files
skill/advanced-patterns.md- Production-ready patterns for:binver, migrations, atomicupgrade+migrate pattern
skill/standards-reference.md- Complete SEP/CAP reference:Updated Files
skill/contracts-soroban.md__constructordefinition, deployment methods (CLI/contract/SDK), rules, andcomparison with initialize pattern
skill/SKILL.mdWhy This Matters
These additions fill critical gaps identified in an audit against
stellar-protocol:Disclaimer
More seps or caps could be added depending on your perspective, although adding the necessary ones would be essential to have a complete overview of everything.