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

Setting Up a Repo Function Which Leverages Raw() to Push Process of Grouping Bets Table to Create a Market:User Map #352

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

pwdel
Copy link
Member

@pwdel pwdel commented Sep 29, 2024

… by building a market:user map and counting all entries within it.
@pwdel pwdel requested review from j4qfrost and ajlacey September 29, 2024 03:53
@pwdel pwdel linked an issue Sep 29, 2024 that may be closed by this pull request
@pwdel pwdel mentioned this pull request Sep 29, 2024
@pwdel pwdel self-assigned this Oct 2, 2024
Comment on lines +30 to +32
usersByMarket := make(map[string][]string)

// create map of market_id's and users for all bets
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd switch the comment and the map making lines, and condense them to a single block.

// comment here
usersByMarket:= ....
for _, bet ...

This logic is all together so grouping them under the comment makes it clear that these go together.

Comment on lines +41 to +45
// Count total first-time bets (users across markets)
var totalFirstTimeBets int64
for _, users := range usersByMarket {
totalFirstTimeBets += int64(len(users))
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this counting unique users per market for all markets? i.e. double counts of users are permitted as long as users are double counted in different markets?

Copy link
Member Author

Choose a reason for hiding this comment

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

Correct!

The reason being, as described above ... https://github.com/openpredictionmarkets/socialpredict/pull/352/files#r1817878774

We're trying to calculate fees.

@@ -0,0 +1,48 @@
package repository
Copy link
Collaborator

Choose a reason for hiding this comment

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

I still find this term repository to be confusing. I've only ever heard it used to refer to a code base not a wrapper for a database. Do you have any examples of it being used this way?

Copy link
Member Author

Choose a reason for hiding this comment

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

Stated this on discord, putting it here as well for transparency.

https://github.com/openpredictionmarkets/socialpredict/pull/352/files ... Here is some more background information on the concept of a repository pattern, specifically in golang. https://github.com/jorzel/go-repository-pattern ... related to this, is the concept of Domain Driven Design (DDD). We are already following a DDD in that our business logic is in Golang and our Application logic is in React. So what the repository pattern is, is an Infrastructure Layer which basically says, in an authoritarian way, "this is how you interact with the database," ... with the idea that, we're designing efficiency from the start, reducing refactoring down the line that we may need to deal with if we consider that our bet table should grow geometrically with the number of users. E.g. the way we are typically doing queries now, is we are pulling the entire bet table, and then having Golang run through that table and having faith that Golang is fast enough to deal with this. So while Golang is extremely powerful, my concern would be if we have a large number of users simultaneously, an event, let's say in the case of Kenyon, election day, and all of those users are pulling the entire table again and again with different functions they are doing, that becomes an I/O problem and a memory problem. Though admittedly as we discussed, I don't know how much of a memory problem that will become. Also admittedly, we could just tell users, "you need to buy more hardware for the time being."

// Execute raw SQL query to group by both market_id and username
// gorm GROUP does not allow two inputs, so this has to be a raw function
result := repo.db.Raw(`
SELECT market_id, username
Copy link
Collaborator

Choose a reason for hiding this comment

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

If we did SELECT DISTINCT .... would that return the desired result?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't believe it does, I remember trying that, would have to go back and check this. I'm trying to get a COUNT value which we can then, "Scan" (using Gorm terminology) into an int64.

return &BetsRepository{db: db}
}

func (repo *BetsRepository) FirstTimeBets() (int64, error) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not really clear on what this is supposed to return/do? Some of my follow on comments are trying to figure that out.

Can you write a comment explaining what this function is supposed to do?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good question. The FirstTimeBets() function is meant to quickly and efficiently count all first time bets on a particular market.

This number, an int64 is then multiplied by the https://github.com/openpredictionmarkets/socialpredict/blob/main/backend/setup/setup.yaml#L16 ... initialBetFee amount to calculate the total number of first time bet fees obtained from the market.

This can be used either in payment to the market maker upon resolution of the market, or within the sitewide stats calculation api which helps ensure that no untracked money is created in the system.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

Data Repository
2 participants