solana-labs / solana-program-library

A collection of Solana programs maintained by Solana Labs
https://solanalabs.com
Apache License 2.0
3.5k stars 2.04k forks source link

Token-swap js bindings don't allow passing curve parameters #2234

Closed ilmoi closed 3 years ago

ilmoi commented 3 years ago

With current js bindings it's not possible to instantiate a Token Swap with curves that require parameters during initilization.

An example is the fixed price curve, which requires a single parameter:

pub struct ConstantPriceCurve {
    /// Amount of token A required to get 1 token B
    pub token_b_price: u64,
}

The layout is correctly planned out to include 32 bytes for curve parameters, but the actual commandDataLayout.encode() call stops just 1 argument short and doesn't include curveParamters. As a result the program always reads them as 0s, which leads to the following error:

'Program log: Error: Given pool token amount results in zero trading tokens'

Which makes sense since the exchange rate is basically set to 0.

The solution would be to simply add a buffered uint 8 array along the lines of:

const encodeLength = commandDataLayout.encode(
        let params = new Uint8Array(32); //<-- 32 byte long array
        //somehow modify it based on user's arguments
        {
          instruction: 0, // InitializeSwap instruction
          nonce,
          tradeFeeNumerator,
          tradeFeeDenominator,
          ownerTradeFeeNumerator,
          ownerTradeFeeDenominator,
          ownerWithdrawFeeNumerator,
          ownerWithdrawFeeDenominator,
          hostFeeNumerator,
          hostFeeDenominator,
          curveType,
          curveParameters: Buffer.from(params), //<-- then add it as the last argument here
        },
        data,
      );
      data = data.slice(0, encodeLength);

And to then also add a new argument for the constructor / init method that asks the user what, if any, curve params they want to pass. I've done a rough test locally and it works.

I'm happy to work on this and submit a PR. I'm just new to solana community / open source and wasn't sure whether it's ok to directly do that or whether I need open an issue first. If someone from the team could comment that'd be great.

joncinque commented 3 years ago

You've got this exactly right, the work was just never done to add this to the JS bindings. Please feel free to add a PR to integrate the curve params! You can tag me in the PR to review whenever youget it done and use this issue to track the work if you like