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

Add an Orca oracle type #813

Merged
merged 22 commits into from
Jan 4, 2024
Merged

Add an Orca oracle type #813

merged 22 commits into from
Jan 4, 2024

Conversation

Lou-Kamades
Copy link
Contributor

No description provided.

@github-actions github-actions bot added the program On-chain program changes label Dec 8, 2023
@Lou-Kamades Lou-Kamades changed the title WIP: Add an Orca oracle type Add an Orca oracle type Dec 9, 2023
Copy link
Contributor

@ckamm ckamm left a comment

Choose a reason for hiding this comment

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

I'll need another more detailed review pass...

programs/mango-v4/src/health/account_retriever.rs Outdated Show resolved Hide resolved
programs/mango-v4/src/health/account_retriever.rs Outdated Show resolved Hide resolved
programs/mango-v4/src/health/account_retriever.rs Outdated Show resolved Hide resolved
let whirlpool = Whirlpool::try_deserialize(&mut &data[..]).unwrap();
require!(
whirlpool.token_mint_a == usdc_mint_mainnet::ID
|| whirlpool.token_mint_b == usdc_mint_mainnet::ID,
Copy link
Contributor

Choose a reason for hiding this comment

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

ah, you flip implicitly based on what USDC is, smart!

programs/mango-v4/src/state/orca_cpi.rs Outdated Show resolved Hide resolved
@ckamm
Copy link
Contributor

ckamm commented Dec 14, 2023

I'd like to see an integration test that uses an orca oracle. It's reasonably easy to do now that ProgramTestContext has set_account(). (our solana.rs has a new set_account() for anchor-style accounts, added fairly recently in dev - but ProgramTestContext::set_account() can also set some bytes you read from a file to a chosen address)

@Lou-Kamades Lou-Kamades force-pushed the lou/orca-oracle branch 7 times, most recently from dcd2abb to 38143d0 Compare December 19, 2023 20:07
programs/mango-v4/src/health/account_retriever.rs Outdated Show resolved Hide resolved
programs/mango-v4/src/state/bank.rs Show resolved Hide resolved
programs/mango-v4/src/state/oracle.rs Outdated Show resolved Hide resolved
programs/mango-v4/src/state/orca_cpi.rs Outdated Show resolved Hide resolved
account.set_data(data);
let mut program_test_context = solana.context.borrow_mut();
program_test_context.set_account(&Pubkey::from_str(fixture.0).unwrap(), &account);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@@ -344,7 +344,7 @@ async fn test_health_compute_tokens_fallback_oracles() -> Result<(), TransportEr
#[tokio::test]
async fn test_health_compute_serum() -> Result<(), TransportError> {
let mut test_builder = TestContextBuilder::new();
test_builder.test().set_compute_max_units(130_500);
test_builder.test().set_compute_max_units(135_000);
Copy link
Contributor

Choose a reason for hiding this comment

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

huh, how did this patch raise the CU cost this much?

@ckamm
Copy link
Contributor

ckamm commented Dec 29, 2023

if the index thing is a real issue, then this code is undertested (and has been before); would much appreciate adding a test that would at least detect essential issues like this one (Resolved: was not a real issue)

Copy link
Contributor

@ckamm ckamm left a comment

Choose a reason for hiding this comment

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

(please squash-merge)

@Lou-Kamades Lou-Kamades merged commit 9ce6b67 into dev Jan 4, 2024
13 checks passed
@Lou-Kamades Lou-Kamades deleted the lou/orca-oracle branch January 4, 2024 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
program On-chain program changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants