osmosis-labs / osmojs

OsmosJS makes it easy to compose and broadcast Osmosis and Cosmos messages
https://cosmology.zone/products/osmojs
Apache License 2.0
63 stars 32 forks source link

Misbehavior With Amino Types #12

Closed DavideSegullo closed 1 year ago

DavideSegullo commented 1 year ago

Hello, I'm using osmojs for sinfonia-ui, but I found two errors inside the AminoConverter:

  1. The amino converter related to /osmosis.lockup.MsgLockTokens should produce as a return for the "toAmino" function an object with the following structure:
    {
    owner: string;
    duration: string;
    coins: Coin[];
    }

Although, it returns an object with the following structure:

{
    owner: string;
    duration: {
        seconds: string;
        nanos: number;
    };
    coins: {
        denom: string;
        amount: string;
    }[];
}

As it's possible to see, here the duration field is returned as an object with seconds and nanos attributes, instead of a single string.

That's how I implemented it on sinfonia: https://github.com/bitsongofficial/sinfonia-ui/blob/9aaa13947b971d220ae64600704645faf2133efb/src/signing/amino-types.ts#L89

  1. The amino type related to the message /osmosis.lockup.MsgBeginUnlocking seems to be wrong, it isn't osmosis/lockup/begin-unlocking as in line 24, since on osmosis it's defined such as osmosis/lockup/begin-unlock-period-lock as in line 14. You could get another reference directly from the osmosis frontend
pyramation commented 1 year ago

thanks! great info. I'm also curious, how are you creating the lockTokens message? In many ways I'm happy to see this issue because I'm trying to get more information about it myself.

related:

DavideSegullo commented 1 year ago

Ye, ofc that's how I create lockTokens message on line 43

pyramation commented 1 year ago

ok so @DavideSegullo as an interface, you're essentially passing in durations as objects. This does make sense.

For context, we used strings because I had originally reverse engineered the code from osmosis-frontend which used strings.

I'll be updating this after HackWasm! Thanks for the details :)

DavideSegullo commented 1 year ago

I add that also the message MsgCreateGauge presents the problem of duration

pyramation commented 1 year ago

Thanks! Will be able to sort this out soon, it's on my todo list!

JoseRFelix commented 1 year ago

Running into this same issue. I'm working around it by sending a plain number instead of a string:

const duration: 126000
{
      /**
       * Marking as 'any' because Duration type definition in telescope is wrong.
       * @see https://github.com/osmosis-labs/osmojs/issues/12
       * @see https://github.com/osmosis-labs/telescope/issues/211
       */
      duration: duration as any,
}
pyramation commented 1 year ago

please try the new release candidate @JoseRFelix @DavideSegullo

osmojs@15.1.0-rc.1

example here https://github.com/cosmology-tech/cosmology/blob/main/packages/cli/src/commands/lock.ts#L113-L116


  const msg = lockTokens({
    owner: account.address,
    coins,
    duration: Duration.fromPartial({
      seconds: Long.fromString(duration),
      nanos: 0
    })
  });