osmosis-labs / osmosis-rust

Rust libraries for osmosis
Apache License 2.0
59 stars 52 forks source link

[Fix] MsgCreateGauge #89

Closed triccs closed 11 months ago

triccs commented 11 months ago

Change pool_id to an Option so you can create gauges ByDuration

triccs commented 11 months ago

@iboss-ptk is there anyway we could get PR #89 & PR #87 merged & released before Oct. 2nd? I'm looking to get the incentives working for a potential launch at that time.

iboss-ptk commented 11 months ago

@iboss-ptk is there anyway we could get PR https://github.com/osmosis-labs/osmosis-rust/pull/89 & PR https://github.com/osmosis-labs/osmosis-rust/pull/87 merged & released before Oct. 2nd? I'm looking to get the incentives working for a potential launch at that time.

That's possible @triccs. For this PR it looks a bit tricky, fixing generated code directly will be overwritten but to automate this, it seems that it's very specific to this field of this msg but for not all u64.

One way I can think of is to modify this transformer to also take src: &Path, descriptor: &FileDescriptorSet, with that we can use get_type_url in the transformer and allow us to specify optional int field (hence empty string) like so

[original]

 transformers::allow_serde_int_as_str(src, s, descriptor, 
   // optional int fields
   vec![
    ("/osmosis.incentives.MsgCreateGauge", vec!["pool_id"])
  ]
)

I can work on this if you want, can get it up before Oct 2nd. Gotta clear some other work first this week :)

triccs commented 11 months ago

@iboss-ptk is there anyway we could get PR #89 & PR #87 merged & released before Oct. 2nd? I'm looking to get the incentives working for a potential launch at that time.

That's possible @triccs. For this PR it looks a bit tricky, fixing generated code directly will be overwritten but to automate this, it seems that it's very specific to this field of this msg but for not all u64.

One way I can think of is to modify this transformer to also take src: &Path, descriptor: &FileDescriptorSet, with that we can use get_type_url in the transformer and allow us to specify optional int field (hence empty string) like so

[original]

 transformers::allow_serde_int_as_str(src, s, descriptor, 
   // optional int fields
   vec![
    ("/osmosis.incentives.MsgCreateGauge", vec!["pool_id"])
  ]
)

I can work on this if you want, can get it up before Oct 2nd. Gotta clear some other work first this week :)

I appreciate it. I definitely thought this would be simple, so thanks for adding it to your list.

iboss-ptk commented 11 months ago

As I investigated this, I don't think it's worth patching the type like this. I think what would make sense is to mark this field optional in proto file.

In the meantime, you can just set pool_id = 0 (ref)

triccs commented 11 months ago

As I investigated this, I don't think it's worth patching the type like this. I think what would make sense is to mark this field optional in proto file.

In the meantime, you can just set pool_id = 0 (ref)

Thank you, my bad then I could've found this.