Skip to content

[Fix] MsgCreateGauge #89

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

Closed
wants to merge 1 commit into from
Closed

[Fix] MsgCreateGauge #89

wants to merge 1 commit into from

Conversation

triccs
Copy link

@triccs triccs commented Sep 14, 2023

Change pool_id to an Option so you can create gauges ByDuration

Change pool_id to an Option<u64> so you can create gauges ByDuration
@triccs
Copy link
Author

triccs commented Sep 18, 2023

@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
Copy link
Collaborator

@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 :)

@triccs
Copy link
Author

triccs commented Sep 19, 2023

@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 iboss-ptk self-assigned this Sep 20, 2023
@iboss-ptk
Copy link
Collaborator

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
Copy link
Author

triccs commented Sep 26, 2023

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.

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