Skip to content
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

feat:multichain contract calls #233

Merged
merged 28 commits into from
Dec 15, 2023
Merged

Conversation

cbrit
Copy link
Member

@cbrit cbrit commented Dec 14, 2023

No description provided.

@cbrit cbrit marked this pull request as ready for review December 14, 2023 18:35
Copy link
Contributor

@EricBolten EricBolten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple comments.

src/cork.rs Outdated
}
let id = id_hash(height, &cellar_id, encoded_call);

let id = id_hash(height, &cellar_id, &encoded_call);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on our conversation we've modified the ID hashes of corks in somm to look like id_hash(height, chain_id, cellar_id, encoded_call), so we'll need to update our calculation here as well.


let mut result: HashMap<u64, HashSet<String>> = HashMap::new();
for id in cork_result.into_inner().cellar_ids {
let normalized_id = match to_checksum_address(&id) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose we can do this for safety's sake, but what we're really concerned with is the data in the incoming request -- the query server on the chain is going to give us strings in the form of checksum addresses.

self.inner_cork.query_scheduled_corks_by_id(request).await
}

pub async fn get_axelar_scheduled_corks_by_id(
Copy link
Contributor

@EricBolten EricBolten Dec 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I saw this and was wondering what called it -- it turns out that I think this PR doesn't include the proposal thread looking at Axelar gov props.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah. I can do it in another PR

Copy link
Contributor

@EricBolten EricBolten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@EricBolten EricBolten merged commit 892eec4 into collin/pubsub Dec 15, 2023
11 checks passed
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