Skip to content

Conversation

@jayantk
Copy link
Contributor

@jayantk jayantk commented Feb 28, 2023

Add some docs to each function to make like easier for our auditors.

I moved one if statement as well because I couldn't find the check and got scared for a second. Turns out we just had it in a weird place. There's a test case for this check which passes before/after the change.

@vercel
Copy link

vercel bot commented Feb 28, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Ignored Deployments
Name Status Preview Updated
example-oracle-amm ⬜️ Ignored (Inspect) Feb 28, 2023 at 8:28PM (UTC)
xc-admin-frontend ⬜️ Ignored (Inspect) Feb 28, 2023 at 8:28PM (UTC)

// other Pyth contracts that may live on the same chain).
if instruction.module != GovernanceModule::Target {
return Err(PythContractError::InvalidGovernancePayload)?;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is the one check that moved. we used to fail to deserialize governance instructions if they pointed to the other module, which doesn't really make sense.

Copy link
Contributor

Choose a reason for hiding this comment

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

what do you mean by other Pyth contracts that may live on the same chain?

is the intention here the generated governance vaa should only run in target chain contract? Was there a problem we faced or is it something we can face in future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good question. The governance message format encompasses all Pyth contracts, and this sort of target chain receiver contract is only one such type. At the moment, we only have one other module (an executor contract on pythnet), but we could add more in the future.

@jayantk jayantk merged commit 66783e5 into main Mar 1, 2023
@jayantk jayantk deleted the cw_audit branch March 1, 2023 16:29
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.

3 participants