-
Notifications
You must be signed in to change notification settings - Fork 12
[capricorn] Upgrade to cosmwasm 1.0.0. beta #48
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
Conversation
ejfitzgerald
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to remove fetch/bridge.wasm and then just build it and attach to part of the release like I have done in: https://github.com/fetchai/contract-reconciliation. Think that is a nicer way to do things
| out_dir.push("schema"); | ||
| create_dir_all(&out_dir).unwrap(); | ||
| remove_schemas(&out_dir).unwrap(); | ||
| /* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will add an item on to the backlog to add this back in at some point
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added back, that wasn't too hard :)
| Some(_) => true, | ||
| None => false, | ||
| } | ||
| store.get(&swap_id.to_be_bytes()).is_some() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
| Ok(r) | ||
| Ok(Response::new() | ||
| .add_attributes(attrs) | ||
| .add_submessages(rtx.messages)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a functional difference right? Did you expect to call .add_message it might not make much of the difference but my recollection is that sub messages are ones where the contract is interested in the response.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I'm forced to use .add_submessages here because of message signature: pub messages: Vec<SubMsg<T>, Global>. Using .add_message result in the trait bound 'cosmwasm_std::CosmosMsg<_>: From<SubMsg>' is not satisfied
| Ok(r) | ||
| Ok(Response::new() | ||
| .add_attributes(attrs) | ||
| .add_submessages(rtx.messages)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comment above
| pub mod msg; | ||
| pub mod state; | ||
|
|
||
| #[cfg(test)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😨 - Nice catch!
|
|
||
| on: [push, pull_request] | ||
| on: | ||
| pull_request: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| pull_request: | |
| pull_request: | |
| - master |
I seem to remember that you need this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure to see why would we want to restrict actions to run only for PR targeting master? Running for any PR seems fine?
ejfitzgerald
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me - I guess we need to sort out the GH actions though because they don't look like they are working currently
No description provided.