v0.4 #246

metaproph3t · 2024-08-29T21:12:54Z

Henry-E · 2024-08-30T05:22:23Z

Reviewing it now. It would be useful to get a short high level overview of some of the changes and the new structure.

Henry-E · 2024-08-30T05:29:21Z

programs/autocrat/src/instructions/initialize_proposal.rs

    )]
    pub base_vault: Account<'info, ConditionalVaultAccount>,
    #[account(
-        constraint = pass_amm.base_mint == base_vault.conditional_on_finalize_token_mint,
-        constraint = pass_amm.quote_mint == quote_vault.conditional_on_finalize_token_mint,
+        constraint = pass_amm.base_mint == base_vault.conditional_token_mints[1],


Instead of using 0 and 1 everywhere i'm wondering if it would be better / possible to use an enum. So it's clear this is referring to pass / fail.

100%, this is a great idea

Addressed by 1b68f4b

programs/conditional_vault/src/error.rs

programs/conditional_vault/src/instructions/add_metadata_to_conditional_tokens.rs

programs/conditional_vault/src/instructions/redeem_tokens.rs

programs/conditional_vault/src/instructions/resolve_question.rs

Henry-E · 2024-08-30T06:37:52Z

programs/autocrat/src/instructions/finalize_proposal.rs

-        let (new_proposal_state, new_vault_state) = if pass_market_twap > threshold {
-            (ProposalState::Passed, VaultStatus::Finalized)
+        let (new_proposal_state, payout_numerators) = if pass_market_twap > threshold {
+            (ProposalState::Passed, vec![0, 1])


ok, i see now that these are the "numerators" that say the proposal 100% passed or 100% failed. On the surface of it, not super intuitive. Don't have an immediate suggestion though.

programs/conditional_vault/src/state/question.rs

92GC · 2024-09-03T15:54:47Z

programs/conditional_vault/src/instructions/resolve_question.rs

+            question.num_outcomes(),
+            VaultError::InvalidNumPayoutNumerators
+        );
+


Should we check that the options add up to 100%:

require_eq!( question.payout_denominator, args.payout_numerators.iter().sum(), VaultError::InvalidNumPayoutNumerators );

this is implicit because we're setting question.payout_denominator equal to this sum, so check not needed

programs/conditional_vault/src/state/question.rs

metaproph3t · 2024-09-06T22:31:27Z

Reviewing it now. It would be useful to get a short high level overview of some of the changes and the new structure.

High-level: abstractify the conditional vault to allow for multiple options and variable payouts for each option. We do this via having "questions" which have "outcomes," which themselves have "payouts." It's pretty similar to the way that Gnosis' conditional token framework works.

programs/conditional_vault/src/instructions/redeem_tokens.rs

Henry-E · 2024-09-07T06:43:25Z

So what's the intention for the multi-outcome markets here?

The conditional vaults can be used to create conditional tokens
There's an oracle that determines the outcome
The AMM still only deals with TWAP proposals
Where does trading for multi-outcome markets actually happen? Is the UI just going to plug into a bunch of regular meteora AMMs or is there another program still to come?

metaproph3t · 2024-09-07T14:25:45Z

So what's the intention for the multi-outcome markets here?

* The conditional vaults can be used to create conditional tokens

* There's an oracle that determines the outcome

* The AMM still only deals with TWAP proposals
  Where does trading for multi-outcome markets actually happen? Is the UI just going to plug into a bunch of regular meteora AMMs or is there another program still to come?

Each outcome will have its own AMM markets, so we don't need to change the amm program to support this. We will need to change autocrat to handle multiple markets (it'll need to receive all of them and pick the option with the highest TWAP).

92GC · 2024-09-09T12:13:12Z

Each outcome will have its own AMM markets, so we don't need to change the amm program to support this.

If we ignored constraints on dev time, IMO it would be better if we used FPMMs like Polymarket and Gnosis. Using separate AMMs is leaky. If I buy Pass I am explicitly selling Fail here. Arb bots will be able to scalp across AMMs here. I guess pro traders can use Jito bundles though.

Using an FPMM would also get rid of the leak in the proposers' liquidity from arbs with the spot market. Though, these arbs are good for the DAO metrics and revenue.

This is audited code for a multi-asset AMM: https://github.com/igneous-labs/s

metaproph3t · 2024-09-09T15:58:37Z

Each outcome will have its own AMM markets, so we don't need to change the amm program to support this.

If we ignored constraints on dev time, IMO it would be better if we used FPMMs like Polymarket and Gnosis. Using separate AMMs is leaky. If I buy Pass I am explicitly selling Fail here. Arb bots will be able to scalp across AMMs here. I guess pro traders can use Jito bundles though.

Using an FPMM would also get rid of the leak in the proposers' liquidity from arbs with the spot market. Though, these arbs are good for the DAO metrics and revenue.

The AMM is actually already a FPMM (more commonly referred to as a CPMM). How would using one stop value leakage through spot market arbing?

92GC · 2024-09-09T18:11:42Z

How would using one stop value leakage through spot market arbing?

Sorry, this doesn't fix arbs with spot. There won't be any arbs with spot for betting markets with PassUSDC/FailUSDC tokens.

Though for DAO related decisions a PassDaoToken / FailDAOToken betting market is more capital efficient for holders.

robre · 2024-09-10T17:28:17Z

programs/autocrat/src/instructions/finalize_proposal.rs

-        has_one = base_vault,
-        has_one = quote_vault,
+        // has_one = base_vault,
+        // has_one = quote_vault,


shouldn't there be a has_one = question now?

this might be a critical issue.

create proposal with question A, and let it pass

create question B with the same proposal as oracle

call finalize_proposal with the proposal, but with question B

question B will now be resolved, and the proposal will be blocked from resolving with question A because the state is now passed/failed

yep, this was indeed a critical issue. resolved: e616e3a

robre · 2024-09-10T17:55:01Z

programs/conditional_vault/src/instructions/initialize_conditional_vault.rs

+            let signer = &[&seeds[..]];
+            let cpi_ctx = CpiContext::new_with_signer(cpi_program, cpi_accounts, signer);
+
+            system_program::create_account(


https://github.com/solana-labs/solana/blob/27eff8408b7223bb3c4ab70523f8a8dca3ca6645/programs/system/src/system_processor.rs#L161
when accounts already have lamports, create_account will fail
attackers may send a lamport to your conditional accounts and make initialization impossible.

that's retarded, okay I guess I will do what anchor does: https://github.com/coral-xyz/anchor/blob/340e9c13f42191a1caa6092d811ed930a64c1f7b/lang/syn/src/codegen/accounts/constraints.rs#L1603-L1627

fixed: 166de60

robre · 2024-09-10T22:05:23Z

programs/conditional_vault/src/instructions/merge_tokens.rs

+
+        ctx.accounts.user_underlying_token_account.reload()?;
+        ctx.accounts.vault_underlying_token_account.reload()?;
+


nice, I like the meticulous checks here

robre · 2024-09-10T22:24:03Z

programs/conditional_vault/src/instructions/redeem_tokens.rs

+                .position(|mint| mint == &conditional_mint.key())
+                .unwrap();
+
+            total_redeemable += ((user_conditional_token_account.amount as u128


this logic means that we're rounding down (e.g. 1000 * 100/900 -> 111). This means the vault will end up with some dust

yep, I'm okay with that

robre · 2024-09-10T22:25:50Z

programs/conditional_vault/src/instructions/redeem_tokens.rs

+
+        for acc in user_conditional_token_accounts.iter_mut() {
+            acc.reload()?;
+            require_eq!(acc.amount, 0, VaultError::AssertFailed);


might as well close the accounts

makes it easier for me to index stuff if they're not closed

robre · 2024-09-10T22:32:55Z

programs/conditional_vault/src/state/conditional_vault.rs

+                .iter()
+                .enumerate()
+                .map(|(i, supply)| {
+                    *supply * question.payout_numerators[i] as u64


in redeem_tokens.rs, you cast to u128 for safe multiplication. I'd recommend doing it here as well

yep, good catch!

fixed: 963ee25

robre

I've reviewed the programs, and added some comments. Most importantly you should add a check when resolving amms/questions (has_one = question), as it may be a critical bug.
Otherwise looks mostly quite solid, just added tiny comments

robre · 2024-09-10T22:36:36Z

ah please note my comments often refer to the code before/after what's shown by github as the context...

0xbigz · 2024-09-12T04:36:41Z

programs/conditional_vault/src/instructions/redeem_tokens.rs

+
+            total_redeemable += ((user_conditional_token_account.amount as u128
+                * question.payout_numerators[payout_index] as u128)
+                / question.payout_denominator as u128) as u64;


in addition to dust from round downs (perhaps using div_ceil for one can mitigate)

doing a mix of account loader Context and (unsafe) math logic within one function. would be great to split up math and state modification logic from solana account loader stuff. this makes rust tests much easier to write and lets you fuzz more easily

div_ceil could cause liabilities to exceed assets as it rounds up. we'd want to use div_floor, which is the same as regular division for unsigned integers: https://doc.rust-lang.org/std/primitive.u32.html#method.div_floor

metaproph3t force-pushed the arbitrary-conditional-vault branch from b2bff55 to cdd9812 Compare August 30, 2024 02:08

Henry-E self-requested a review August 30, 2024 05:15

Henry-E reviewed Aug 30, 2024

View reviewed changes

programs/conditional_vault/src/error.rs Show resolved Hide resolved

Henry-E reviewed Aug 30, 2024

View reviewed changes

programs/conditional_vault/src/instructions/add_metadata_to_conditional_tokens.rs Show resolved Hide resolved

Henry-E reviewed Aug 30, 2024

View reviewed changes

programs/conditional_vault/src/instructions/redeem_tokens.rs Show resolved Hide resolved

Henry-E reviewed Aug 30, 2024

View reviewed changes

programs/conditional_vault/src/instructions/resolve_question.rs Show resolved Hide resolved

Henry-E reviewed Aug 30, 2024

View reviewed changes

programs/conditional_vault/src/state/question.rs Show resolved Hide resolved

92GC reviewed Sep 3, 2024

View reviewed changes

92GC reviewed Sep 6, 2024

View reviewed changes

programs/conditional_vault/src/state/question.rs Show resolved Hide resolved

Henry-E reviewed Sep 7, 2024

View reviewed changes

programs/conditional_vault/src/instructions/redeem_tokens.rs Show resolved Hide resolved

metaproph3t requested a review from 0xbigz September 7, 2024 14:26

robre reviewed Sep 10, 2024

View reviewed changes

robre suggested changes Sep 10, 2024

View reviewed changes

0xbigz reviewed Sep 12, 2024

View reviewed changes

Add Question and NewConditionalVault structs

3cc8d29

advaith101 and others added 26 commits October 11, 2024 00:00

wip

0a46925

scalarMarketsTest

74a1372

Add calculate optimal swap for merge

442510b

testing with different scalars

9745c8a

added withdraw liquidity

e20028e

uncommented stuff

3597544

Remove unnecessary function

dc349da

v0.4.0-alpha.17

455202d

closed form solution for optimal swap

8d7a45c

one more

76581c4

explain math

d45be4b

Test the optimal swap and merge

e8994e4

v0.4.0-alpha.18

c4aef57

Optimize calculateOptimalSwapForMerge

a00e367

tests weird amounts for swap/merge optimal amt

0d5fa90

whoops. smol fix

95e094b

removed console.logs

7b8b360

Remove old math for calculateOptimalSwapForMerge

21852ab

v0.4.0-alpha.19

808f901

slippage

ab2404d

v0.4.0-alpha.20

40de26a

Use latest SDK version from tests

cb2b3b5

Bump autocrat to v0.4

f8cf589

Get rid of unnecessary console.logs

96ac995

Update autocrat version

e5243ec

Update autocrat program ID

817ea2e

metaproph3t force-pushed the arbitrary-conditional-vault branch 2 times, most recently from 90a4b2e to 40de26a Compare October 11, 2024 16:05

metaproph3t marked this pull request as ready for review October 11, 2024 16:05

metaproph3t merged commit b4baaca into develop Oct 11, 2024
3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

v0.4 #246

v0.4 #246

metaproph3t commented Aug 29, 2024

Henry-E commented Aug 30, 2024 •

edited

Loading

Henry-E Aug 30, 2024 •

edited

Loading

metaproph3t Aug 30, 2024

metaproph3t Sep 6, 2024

Henry-E Aug 30, 2024

92GC Sep 3, 2024

metaproph3t Sep 6, 2024

metaproph3t commented Sep 6, 2024

Henry-E commented Sep 7, 2024

metaproph3t commented Sep 7, 2024

92GC commented Sep 9, 2024 •

edited

Loading

metaproph3t commented Sep 9, 2024

92GC commented Sep 9, 2024 •

edited

Loading

robre Sep 10, 2024

robre Sep 10, 2024

metaproph3t Sep 13, 2024

robre Sep 10, 2024

metaproph3t Sep 13, 2024

metaproph3t Sep 13, 2024

robre Sep 10, 2024

robre Sep 10, 2024

metaproph3t Sep 13, 2024

robre Sep 10, 2024

metaproph3t Sep 13, 2024

robre Sep 10, 2024

metaproph3t Sep 13, 2024

metaproph3t Sep 13, 2024

robre left a comment

robre commented Sep 10, 2024

0xbigz Sep 12, 2024

metaproph3t Sep 13, 2024


		ctx.accounts.user_underlying_token_account.reload()?;
		ctx.accounts.vault_underlying_token_account.reload()?;

v0.4 #246

v0.4 #246

Conversation

metaproph3t commented Aug 29, 2024

Henry-E commented Aug 30, 2024 • edited Loading

Henry-E Aug 30, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

metaproph3t commented Sep 6, 2024

Henry-E commented Sep 7, 2024

metaproph3t commented Sep 7, 2024

92GC commented Sep 9, 2024 • edited Loading

metaproph3t commented Sep 9, 2024

92GC commented Sep 9, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

robre left a comment

Choose a reason for hiding this comment

robre commented Sep 10, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Henry-E commented Aug 30, 2024 •

edited

Loading

Henry-E Aug 30, 2024 •

edited

Loading

92GC commented Sep 9, 2024 •

edited

Loading

92GC commented Sep 9, 2024 •

edited

Loading