Skip to content

Conversation

@pwdel
Copy link
Member

@pwdel pwdel commented Apr 26, 2025

  • calculateDBPMP was outputting payouts to both winners and losers.
  • Adding test functionality for proof of fix
  • This change may have been superseded by: Refactoring Payout #431

Works on Local after testing, all tests passed.

image

Upon further analysing and comparing to 432 which had already been merged into main, calculateDBPMPayouts should be merged and used because:

Branch Comparison: payoutbyratio vs calculatedbpmpayouts


1 Structural / Directory-Level Differences

🔹 Only in calculatedbpmpayouts

New / moved package / file Purpose (quick read)
handlers/math/payout/* Payout core moved out of handlers/markets/; exposes DistributePayoutsWithRefund, splits logic further.
handlers/math/positions/* Centralises share-count ↔ valuation math; adds rounding-correction helper.
security/* (ratelimit.go, headers.go, sanitizer.go + tests) Mini security middleware: header hardening, simple rate-limit, HTML/JS sanitiser.
handlers/users/apply_transaction.go Single entry for credit/debit with enums (TransactionWin, etc.).
Several README_*.md files Dev notes for math & users modules.
Extra bot / WPAM tests (wpam_current_test.go) Unit tests for the probability algorithm.

🔹 Only in payoutbyratio

File Purpose
handlers/markets/resolvemarketpayouts.go Ratio-based payout (SelectWinningPositions, AllocateWinningSharePool).
handlers/markets/resolvemarketcore.go Thin wrapper that calls the above and updates balances in-place.
handlers/users/updatebalanceafterpayout.go Direct balance-mutation helper.
(draft) handlers/math/financials/calculateprofit.draft.go Early stub for profit calculations.

Take-away: calculatedbpmpayouts is much more than just “DBPM payout.”
It introduces a security layer, new math/positions abstraction, extra tests, and rewires payout logic into its own package.


2 Logic Overlap vs. Divergence

Concern payoutbyratio branch calculatedbpmpayouts branch
Payout algorithm Distributes market volume pro-rata to winning shares (ratio = userShares / totalWinningShares). Calls positionsmath.CalculateValuations_WPAM_DBPM() first, then pays out integer valuations. Also refunds on “N/A” resolutions.
Payout-code location Lives under handlers/markets/; touches HTTP handler directly. Lives under handlers/math/payout/; markets handler merely orchestrates → test-friendly.
User balance updates Helper updatebalanceafterpayout.go mutates balances directly. ApplyTransactionToUser() centralises balance changes & labels them (TransactionWin, TransactionRefund, …).
Safety / error handling Minimal checks (nil market, zero shares). Adds explicit nil checks, DB tx rollbacks, structured logs.
Tests 4 payout-focused unit tests. 12 payout/position tests plus security & WPAM tests.

3 What’s Already on main

  • Ratio-based payout (handlers/markets/*).
  • No security/* middleware.
  • No handlers/math/payout/* separation.
  • Fewer unit tests around payout/positions.

@pwdel pwdel added the bug Something isn't working label Apr 26, 2025
@pwdel pwdel self-assigned this Apr 26, 2025
@pwdel pwdel requested review from ajlacey and astrosnat April 27, 2025 13:37
Copy link
Collaborator

@astrosnat astrosnat left a comment

Choose a reason for hiding this comment

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

LGTM but why are some of the backend tests failing?

@pwdel
Copy link
Member Author

pwdel commented Apr 28, 2025

LGTM but why are some of the backend tests failing?

Which ones @astrosnat ? Can you paste the results you are getting?

@pwdel
Copy link
Member Author

pwdel commented Apr 28, 2025

@astrosnat

Whoops, didn't realize results are in the pipeline now.

ok  	socialpredict/handlers/positions	0.050s
--- FAIL: TestGetSetupHandler (0.00s)
    --- FAIL: TestGetSetupHandler/successful_load (0.00s)
        setuphandler_test.go:73: successful load: handler returned unexpected body: got {"MarketCreation":{"InitialMarketProbability":0.5,"InitialMarketSubsidization":10,"InitialMarketYes":0,"InitialMarketNo":0},"MarketIncentives":{"CreateMarketCost":10,"TraderBonus":1},"User":{"InitialAccountBalance":0,"MaximumDebtAllowed":500},"Betting":{"MinimumBet":1,"BetFees":{"InitialBetFee":1,"BuySharesFee":0,"SellSharesFee":0}}}
             want {
            				"MarketCreation":{"InitialMarketProbability":0.5,"InitialMarketSubsidization":10,"InitialMarketYes":0,"InitialMarketNo":0},
            				"MarketIncentives":{"CreateMarketCost":10,"TraderBonus":1},
            				"User":{"InitialAccountBalance":0,"MaximumDebtAllowed":500},
            				"Betting":{"MinimumBet":1,"BetFees":{"InitialBetFee":1,"EachBetFee":0,"SellSharesFee":0}}}
FAIL
FAIL	socialpredict/handlers/setup	0.004s
ok  	socialpredict/handlers/stats	0.006s [no tests to run]

I'll look into this.

@pwdel pwdel marked this pull request as draft May 11, 2025 12:09
@pwdel
Copy link
Member Author

pwdel commented Jul 25, 2025

Need to fix tests then can merge

@pwdel pwdel changed the title Fixing calculateDBPMPayouts with Test Functionality 01 Fixing calculateDBPMPayouts with Test Functionality Jul 25, 2025
@pwdel
Copy link
Member Author

pwdel commented Jul 29, 2025

Finished this set of features so I'm going to try to merge main back into this branch here above. #438

@pwdel
Copy link
Member Author

pwdel commented Jul 29, 2025

/draft

@pwdel pwdel marked this pull request as ready for review July 29, 2025 22:34
@pwdel
Copy link
Member Author

pwdel commented Jul 29, 2025

We need to do usability testing on this @astrosnat because I had noted that this may have been superseded by:

#431

Basically we should see if it gives us the results we are looking for.

@pwdel pwdel merged commit 76e78b5 into main Jul 30, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

Development

Successfully merging this pull request may close these issues.

3 participants