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

Switch pull oracles #231

Merged
merged 11 commits into from
Aug 15, 2024
Merged

Switch pull oracles #231

merged 11 commits into from
Aug 15, 2024

Conversation

jgur-psyops
Copy link
Contributor

@jgur-psyops jgur-psyops commented Aug 7, 2024

Done:

  • Support for switch pull oracles when configuring a bank
  • Test fixture boilerplate
  • permissioned ix to migrate existing SB bank to pull oracle
  • validate staleness when loading feeds
  • validate get_price returns accurate values
  • confidence interval for pull oracles (based on std_dev now)
  • test borrows/accounting

Maybe:

  • lite version of Price Feed
  • flag on banks to indicate migration complete

@jkbpvsc
Copy link
Member

jkbpvsc commented Aug 8, 2024

permissioned ix to migrate existing SB bank to pull oracle

The bank_update instruction should already permit migrating oracles

lite version of Price Feed

This is only relevant if any structs blow the stack, or incur significant compute unit cost to use.

flag on banks to indicate migration complete

There should be no need to migrate, we should just add a new oracle type and then update the bank config to use the new oracle.

@jgur-psyops jgur-psyops changed the title [WIP] Switch pull oracles Switch pull oracles Aug 10, 2024
@jgur-psyops
Copy link
Contributor Author

jgur-psyops commented Aug 10, 2024

Ready to go, except could use a hand writing tests for borrow (e.g. marginfi_account_borrow_failure_not_enough_collateral) Not sure what currency deposit_amount/borrow_amount are denominated in

jkbpvsc
jkbpvsc previously approved these changes Aug 12, 2024
@@ -178,3 +178,17 @@ fn ceil_div(numerator: u128, denominator: u128) -> Option<u128> {
.checked_sub(1)?
.checked_div(denominator)
}

/// A minimal tool to convert a hex string like "22f123639" into the byte equivalent.
pub fn hex_to_bytes(hex: &str) -> Vec<u8> {
Copy link
Member

Choose a reason for hiding this comment

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

Is this required in onchain code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, just for tests. Could move it into the tests where it's needed.

Copy link
Member

Choose a reason for hiding this comment

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

Lets move it into tests yes!

programs/marginfi/src/state/price.rs Outdated Show resolved Hide resolved
programs/marginfi/src/state/price.rs Show resolved Hide resolved
programs/marginfi/src/state/marginfi_account.rs Outdated Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

We can probably just change the main test script to pass additional args to the test command.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No strong feelings either way, I have some parsing at the end there to quickly run a rest with minimal terminal output.

// From mainnet: https://solana.fm/address/BSzfJs4d1tAkSDqkepnfzEVcx2WtDVnwwXa2giy9PLeP
// Actual price $155.59404527
// conf/Std_dev ~5%
let bytes = hex_to_bytes("c41b6cc40ad7db286f5e7566ac000a9530e56b1db49585772719aeaaeeadb4d9bd8c2357b88e9e782e53d81000000000000000000000000000985f538057856308000000000000005cba953f3f15356b17703e554d3983801916531d7976aa424ad64348ec50e4224650d81000000000000000000000000000a0d5a780cc7f580800000000000000a20b742cedab55efd1faf60aef2cb872a092d24dfba8a48c8b953a5e90ac7bbf874ed81000000000000000000000000000c04958360093580800000000000000e7ef024ea756f8beec2eaa40234070da356754a8eeb2ac6a17c32d17c3e99f8ddc50d81000000000000000000000000000bc8739b45d215b0800000000000000e3e5130902c3e9c27917789769f1ae05de15cf504658beafeed2c598a949b3b7bf53d810000000000000000000000000007cec168c94d667080000000000000020e270b743473d87eff321663e267ba1c9a151f7969cef8147f625e9a2af7287ea54d81000000000000000000000000000dc65eccc174d6f0800000000000000ab605484238ac93f225c65f24d7705bb74b00cdb576555c3995e196691a4de5f484ed8100000000000000000000000000088f28dc9271d59080000000000000015196392573dc9043242716f629d4c0fb93bc0cff7a1a10ede24281b0e98fb7d5454d810000000000000000000000000000441a10ca4a268080000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000048ac38271f28ab1b12e49439bddf54871094e4832a56c7a8ec57bd18d357980086807068432f186a147cf0b13a30067d386204ea9d6c8b04743ac2ef010b07524c935636f2523f6aeeb6dc7b7dab0e86a13ff2c794f7895fc78851d69fdb593bdccdb36600000000000000000000000000e40b540200000001000000534f4c2f55534400000000000000000000000000000000000000000000000000000000019e9eb66600000000fca3d11000000000000000000000000000000000000000000000000000000000000000000000000000dc65eccc174d6f0800000000000000006c9225e039550300000000000000000070d3c6ecddf76b080000000000000000d8244bc073aa060000000000000000000441a10ca4a268080000000000000000dc65eccc174d6f08000000000000000200000000000000ea54d810000000005454d81000000000ea54d81000000000fa0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000");
Copy link
Member

Choose a reason for hiding this comment

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

Lets load this from a file using include_bytes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

include_bytes comes from the test-utils program so how should we handle that? Could dupe the method locally to avoid confusion.

Also the test files are currently a few directories away from this locally (../../../test-utils/data), so duplicate them or use the relative path to the current folder?

@@ -1021,4 +1184,92 @@ mod tests {
.unwrap()
);
}
Copy link
Member

Choose a reason for hiding this comment

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

Lets add some tests that test the oracle price parsing and all cases with or without wide price bands

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You mean prices with PriceBias::Low/High? Have that included at the end of test swb_pull_get_price.

@jkbpvsc
Copy link
Member

jkbpvsc commented Aug 12, 2024

Lets also make the CI happy.

@jgur-psyops
Copy link
Contributor Author

jgur-psyops commented Aug 12, 2024

Lets also make the CI happy.

Linter I can do, unit test CI seems to failing on an upstream build issue, the build-and-test CI is the failing test I need some help with (understanding the financials intended to be tested here).

Done except for unit test, where CI seems to failing on an upstream build issue,

@@ -178,3 +178,17 @@ fn ceil_div(numerator: u128, denominator: u128) -> Option<u128> {
.checked_sub(1)?
.checked_div(denominator)
}

/// A minimal tool to convert a hex string like "22f123639" into the byte equivalent.
pub fn hex_to_bytes(hex: &str) -> Vec<u8> {
Copy link
Member

Choose a reason for hiding this comment

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

Lets move it into tests yes!

@jkbpvsc
Copy link
Member

jkbpvsc commented Aug 15, 2024

Lets also make the CI happy.

Linter I can do, unit test CI seems to failing on an upstream build issue, the build-and-test CI is the failing test I need some help with (understanding the financials intended to be tested here).

Done except for unit test, where CI seems to failing on an upstream build issue,

Maybe we should make the unit test command use --locked so it doesn't pull in a new broken package.

@jgur-psyops jgur-psyops merged commit 9eae066 into main Aug 15, 2024
3 of 4 checks passed
@jgur-psyops jgur-psyops deleted the switch-pull-oracles branch August 15, 2024 17:11
jkbpvsc pushed a commit that referenced this pull request Oct 15, 2024
Adds support for switch pull oracles (aka sb-on-demand) when configuring a bank.
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