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

Migrate /rewards routes #604

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open

Migrate /rewards routes #604

wants to merge 4 commits into from

Conversation

adam-gf
Copy link
Contributor

@adam-gf adam-gf commented Dec 23, 2024

Description

  • GET /rewards/budget/{address}/epoch/{epoch}
  • GET /rewards/budget/{address}/upcoming
  • GET /rewards/budgets/epoch/{epoch}
  • POST /rewards/estimated_budget/by_days
  • POST /rewards/estimated_budget
  • GET /rewards/leverage/{epoch}
  • GET /rewards/merkle_tree/{epoch}
  • GET /rewards/projects/epoch/{epoch}
  • GET /rewards/projects/estimated
  • GET /rewards/threshold/{epoch}
  • GET /rewards/unused/{epoch}

@adam-gf adam-gf self-assigned this Jan 14, 2025
@adam-gf adam-gf marked this pull request as ready for review January 16, 2025 00:28
@@ -12,7 +12,9 @@ def get_blocks_range(
if not with_block_range:
return None, None

start_block = get_block_num_from_ts(start_sec) if start_sec <= now_sec else None
end_block = get_block_num_from_ts(end_sec) if end_sec <= now_sec else None
# start_block = get_block_num_from_ts(start_sec) if start_sec <= now_sec else None
Copy link
Contributor

Choose a reason for hiding this comment

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

it should be uncommented, right?

SQLALCHEMY_DATABASE_URI = f"sqlite:///{DB_PATH}"
# SQLALCHEMY_DATABASE_URI = f"sqlite:///{DB_PATH}"
SQLALCHEMY_DATABASE_URI = (
f"postgresql://postgres:mysecretpassword@localhost:5433/postgres"
Copy link
Contributor

Choose a reason for hiding this comment

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

let's remember to restore it as well

@@ -8,4 +8,5 @@ class SmartContract:
def __init__(self, w3: AsyncWeb3, abi: ABI, address: ChecksumAddress) -> None:
self.abi = abi
self.w3 = w3
self.address = address
Copy link
Contributor

Choose a reason for hiding this comment

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

why is it needed here in the base class? is it used by all of children?

@@ -107,6 +110,10 @@ class ChainSettings(OctantSettings):
description="The chain id to use for the signature verification.",
)

@property
def is_mainnet(self) -> bool:
Copy link
Contributor

Choose a reason for hiding this comment

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

i introduced compare_blockchain_types in my previous PR that is placed in the core; it should be unified to either my own way or yours for handling this

Comment on lines +39 to +123
class DepositEventsRepository:
def __init__(
self,
session: AsyncSession,
epochs_subgraph: EpochsSubgraph,
sablier_subgraph: SablierSubgraph,
sablier_unlock_grace_period: int,
):
self.session = session
self.epochs_subgraph = epochs_subgraph
self.sablier_subgraph = sablier_subgraph
self.sablier_unlock_grace_period = sablier_unlock_grace_period

async def get_all_users_events(
self,
epoch_number: int,
start_sec: int,
end_sec: int,
) -> dict[str, list[DepositEvent]]:
"""
Returns all user events (LOCK, UNLOCK) for a given epoch.
"""

# Get all locked amounts for the previous epoch
epoch_start_locked_amounts = await get_all_deposit_events_for_epoch(
self.session, epoch_number - 1
)
epoch_start_events = [
DepositEvent(
user=user,
type=EventType.LOCK,
timestamp=start_sec,
amount=0, # it is not a deposit in fact
deposit_before=int(deposit.epoch_end_deposit),
)
for user, deposit in epoch_start_locked_amounts.items()
]

sablier_streams = await self.sablier_subgraph.get_all_streams_history()
mapped_streams = process_to_locks_and_unlocks(
sablier_streams, from_timestamp=start_sec, to_timestamp=end_sec
)
# print("Mapped streams", mapped_streams)
epoch_events = []
epoch_events += flatten_sablier_events(mapped_streams, FlattenStrategy.ALL)
epoch_events += await self.epochs_subgraph.fetch_locks_by_timestamp_range(
start_sec, end_sec
)
epoch_events += await self.epochs_subgraph.fetch_unlocks_by_timestamp_range(
start_sec, end_sec
)

epoch_events = [DepositEvent.from_dict(event) for event in epoch_events]
sorted_events = sorted(epoch_events, key=attrgetter("user", "timestamp"))

user_events = {
k: list(g) for k, g in groupby(sorted_events, key=attrgetter("user"))
}

for event in epoch_start_events:
if event.user in user_events:
user_events[event.user].insert(0, event)
else:
user_events[event.user] = [event]

epoch_start_users = list(map(attrgetter("user"), epoch_start_events))
for user_address in user_events:
if user_address not in epoch_start_users:
user_events[user_address].insert(
0,
DepositEvent(
user_address,
EventType.LOCK,
timestamp=start_sec,
amount=0,
deposit_before=0,
),
)

user_events[user_address] = unify_deposit_balances(
user_events[user_address],
self.sablier_unlock_grace_period,
)

return user_events
Copy link
Contributor

@kgarbacinski kgarbacinski Jan 17, 2025

Choose a reason for hiding this comment

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

for me this is more a part of service, isn't it? maybe let's try to keep it consistent and keep the repository as a database layer only with the minimum of business logic?

raise exceptions.EpochNotIndexed(epoch_number)

# Parse response and return result
logging.debug(f"[Subgraph] Received epoch properties: {data[0]}")
return TypeAdapter(EpochSubgraphItem).validate_python(data[0])
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe let's create an alias for that?

Copy link
Contributor

@kgarbacinski kgarbacinski Jan 17, 2025

Choose a reason for hiding this comment

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

e.g similar to transform_to_checksum_address

@@ -5,11 +5,11 @@

from v2.allocations.schemas import AllocationWithUserUQScore
from v2.core.types import Address
from v2.project_rewards.schemas import ProjectFundingSummary
from v2.project_rewards.schemas import ProjectFundingSummaryV1


class CappedQuadriaticFunding(NamedTuple):
Copy link
Contributor

Choose a reason for hiding this comment

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

typo: Quadriatic --> Quadratic



class CappedQuadriaticFunding(NamedTuple):
project_fundings: dict[Address, ProjectFundingSummary]
project_fundings: dict[Address, ProjectFundingSummaryV1]
amounts_total: Decimal # Sum of all allocation amounts for all projects
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
amounts_total: Decimal # Sum of all allocation amounts for all projects
allocations_total_for_all_projects: Decimal # Sum of all allocation amounts for all projects



class CappedQuadriaticFunding(NamedTuple):
project_fundings: dict[Address, ProjectFundingSummary]
project_fundings: dict[Address, ProjectFundingSummaryV1]
amounts_total: Decimal # Sum of all allocation amounts for all projects
matched_total: Decimal # Sum of all matched rewards for all projects
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
matched_total: Decimal # Sum of all matched rewards for all projects
matched_total_for_all_projects: Decimal # Sum of all matched rewards for all projects


@property
def lock_duration_sec(self) -> int:
return self.days * 86400 # 24hours * 60minutes * 60seconds
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return self.days * 86400 # 24hours * 60minutes * 60seconds
return self.days * 24 * 60 * 60

rewards = calculate_rewards(settings, eth_proceeds, total_effective_deposit)

# fmt: off
return OctantRewardsDTO(
Copy link
Contributor

Choose a reason for hiding this comment

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

fancy but is it worth to turn off the formatter to have these values only justified in the code? 🚀

Copy link
Contributor

Choose a reason for hiding this comment

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

this is a part of service, right? we do we keep it in the separate file user_events? if services.py is growing exponentially, let's maybe try to create a package services and create separate domain-related files out there? you can take a look at the projects package

Copy link
Contributor

Choose a reason for hiding this comment

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

we'll also need to merge #600 to this one (there are some changes for getting streams for the giveaway winners)

}

# Cache into file to avoid rate limiting
if not os.path.exists("sablier_streams.json"):
Copy link
Contributor

Choose a reason for hiding this comment

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

it's very risky to cache it this way; we use sablier for computing effective_deposit just before every AW; if user does any single operation (like unlocking) just before the AW, we're in the deep ass because the result are not up-to-date with the real state, right?

Copy link
Contributor

@kgarbacinski kgarbacinski left a comment

Choose a reason for hiding this comment

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

a few minor issues but also blockers to address

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