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

Add READMEs for SDKs on npmjs and crates #449

Merged
merged 10 commits into from
Nov 5, 2024

Conversation

calintje
Copy link
Contributor

@calintje calintje commented Nov 3, 2024

Title
Add Updated READMEs for Orca Whirlpools SDKs

Details

  • Added four updated README files for different Orca Whirlpools SDKs:
    • Rust: low level whirlpool client based on Codama-IDL
    • Rust: core library for math and quotes
    • TS: low level whirlpool client based on Codama-IDL
    • TS: core library for math and quotes, written in Rust, compiled to Wasm
  • All code examples are tested
  • All documentation includes installation instructions, feature breakdowns, usage examples, and any notable compatibility or compilation notes for the SDKs.

Rustdoc example

Screenshot 2024-11-04 011709

Typedoc example

Screenshot 2024-11-04 011814

@wjthieme
Copy link
Member

wjthieme commented Nov 3, 2024

I think some of these also show up in the rustdoc/typedoc so worth checking if they look good there as well

@calintje calintje marked this pull request as ready for review November 4, 2024 00:29
Copy link
Member

@wjthieme wjthieme left a comment

Choose a reason for hiding this comment

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

I added a couple of comments to some files but the same applies to the other files where we use the same wording.

I think there are 2 more readme files that show up in the typedoc/rustdoc

  • docs/rust/README.md
  • docs/ts/README.md

Are you planning to add these in this PR also?

## Overview
This package provides developers with low-level functionalities for interacting with the Whirlpool Program on Solana. It serves as a foundational tool that allows developers to manage and integrate detailed operations into their Solana projects, particularly those related to Orca's Whirlpool Program. This package offers granular control for advanced use cases.

> NOTE: To ensure compatibility, use version 1.17.22 of the `solana-sdk` crate, which matches the version used to build the Whirlpool program.
Copy link
Member

Choose a reason for hiding this comment

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

The client might be built with solana-sdk v1.18. Ideally we'd want it to work with a whole range of versions instead of just a single one


## Key Features
- **Codama IDL Integration**: The package includes a set of generated client code based on Codama IDL, which provides a standardized representation of the Whirlpool Program. This ensures all the necessary program information is easily accessible in a structured format. It handles all decoding and encoding of instructions and account data, making it much easier to interact with the program.
- **PDA (Program Derived Addresses) Utilities**: This feature contains utility functions that help derive Program Derived Addresses (PDAs) for accounts within the Whirlpool Program, simplifying address generation for developers.
Copy link
Member

Choose a reason for hiding this comment

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

generation -> derivation


fn main() {
let whirlpool_config_address = Pubkey::try_from("FcrweFY1G9HJAHG5inkGB6pKg1HZ6x9UC2WioAfWrGkR").unwrap();
let token_mint_a = Pubkey::try_from("So11111111111111111111111111111111111111112").unwrap(); // wSOL
Copy link
Member

Choose a reason for hiding this comment

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

from_str instead of try_from and then you need to add the use std::str::FromStr;

@@ -1 +1,81 @@
# Orca Whirlpools Rust Client
# Whirlpools Program Client - Low-Level SDK
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to add that this is low-level sdk in the title?

> **Note:** This SDK uses Solana Web3.js SDK v2, which is currently in Release Candidate (RC) status. It is not compatible with the widely used v1.x.x version.

## Key Features
- **Codama IDL Integration**: The package includes a set of generated client code based on Codama IDL, which provides a standardized representation of the Whirlpool Program. This ensures all the necessary program information is easily accessible in a structured format and handles all decoding and encoding of instructions and account data, making it much easier to interact with the program.
Copy link
Member

Choose a reason for hiding this comment

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

Codama IDL -> just a normal IDL so we can remove Codama here. Codama is the generator that consumes the IDL

@@ -1 +1,71 @@
# Orca Whirlpools Typescript Core
# Orca Whirlpools Core SDK (WebAssembly)
This package provides developers with advanced functionalities for interacting with the Whirlpool Program on Solana. Originally written in Rust, it has been compiled to WebAssembly (Wasm). This compilation makes the SDK accessible in JavaScript/TypeScript environments, offering developers the same core features and calculations for their Solana projects. The SDK exposes convenient methods for math calculations, quotes, and other utilities, enabling seamless integration within web-based projects.
Copy link
Member

Choose a reason for hiding this comment

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

Solana projects -> typescript projects.


## Key Features
- **WebAssembly Integration**: The original Rust-based SDK is compiled to Wasm, allowing it to be used in JavaScript and TypeScript projects for easy integration with web-based environments.
- **Math Library**: Contains a variety of functions for math operations related to bundles, positions, prices, ticks, and tokens, including calculations such as determining position status or price conversions.
Copy link
Member

Choose a reason for hiding this comment

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

Think we can remove WebAssembly Integration here since the Math library and the Quoting library are the webassembly integration.

I don't think it is important for a consumer to know that under-the-hood we use wasm though so we might consider making it less prominent (but not remove completely since you need to be using a version of node/web/bundler that supports wasm)

@calintje
Copy link
Contributor Author

calintje commented Nov 4, 2024

I added a couple of comments to some files but the same applies to the other files where we use the same wording.

I think there are 2 more readme files that show up in the typedoc/rustdoc

  • docs/rust/README.md
  • docs/ts/README.md

Are you planning to add these in this PR also?

Thank you for the review. I'll have a look at the comments shortly.

I'd like to get this PR through ASAP, prioritizing the publishing of the READMEs on npmjs and crates. I will open another PR for the other two docs you mentioned, together with a more general overview of our SDK offering. Does that sound good to you?

@wjthieme
Copy link
Member

wjthieme commented Nov 4, 2024

I'd like to get this PR through ASAP, prioritizing the publishing of the READMEs on npmjs and crates. I will open another PR for the other two docs you mentioned, together with a more general overview of our SDK offering. Does that sound good to you?

Yep that sounds good

Copy link
Member

@wjthieme wjthieme left a comment

Choose a reason for hiding this comment

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

Two minor details but LGTM!

let (fee_tier, _bump) = get_fee_tier_address(&whirlpool_config_address, tick_spacing).unwrap();
let initial_sqrt_price = 7459106261056563200u128;

let initialize_pool_instruction = InitializePoolBuilder::new()
Copy link
Member

Choose a reason for hiding this comment

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

Should we use v2 here?

const initialSqrtPrice = BigInt(7459106261056563200);

// Create InitializePool instruction
const initializePoolInstruction = getInitializePoolInstruction({
Copy link
Member

Choose a reason for hiding this comment

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

V2?

@calintje calintje merged commit a418902 into main Nov 5, 2024
5 checks passed
@calintje calintje deleted the calintje/readmes-sdks-npmjs-crates branch November 5, 2024 11:39
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.

2 participants