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.

Sign up for GitHub

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

Fix sample rewards script #805

Merged
merged 4 commits into from
Nov 24, 2023
+44 −37
Merged

Fix sample rewards script #805

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8b0d92d
use estimated proposal pass time to calculate delay_blocks
teddyding Nov 24, 2023
77d9b0f
fix comments
teddyding Nov 24, 2023
be6480b
update default time
teddyding Nov 24, 2023
386a4f6
address comments
teddyding Nov 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 44 additions & 37 deletions protocol/scripts/governance/bridge_vesters_set_rewards.py
View file
Original file line number Diff line number Diff line change
@@ -1,12 +1,17 @@
import json
from dateutil import parser
from datetime import datetime, timedelta
import pytz
import sys

NINE_ZEROS="000000000"


# Usage:
# 1. Update below section with appropriate values
# 2. Run `python3 bridge_vesters_set_rewards.py`
# 3. Use generated `.json` file submitting proposal. This should be done within a few hours after
# the script is generated so that the estimated delay_blocks are accurate.


##########################################################################
@@ -34,13 +39,20 @@
REWARDS_MULTIPLIER_UPDATE_2=900000 # in parts-per-million (example given: 0.9)
# TODO: update time
UPDATE_2_TIME_UTC="2025-12-01T15:00:00+00:00"
# TODO: update as needed. Can be checked in Mintscan. Value range for reference: 1.2 < X < 1.35
AVG_BLOCK_TIME_FOR_ESTIMATE=1.3
# TODO: update as needed. Can be checked in Mintscan by visiting a future block:
# `www.mintscan.io/chain_name/block/9000000`
# This can fluctuate; value range for reference: 1.2 <= X <= 1.3
AVG_BLOCK_TIME_FOR_ESTIMATE=1.25
##########################################################################
### END: Required proposal fields. ###
### TODO: update above fields as needed ###
##########################################################################

##########################################################################
### Network specific constants ###
### Only change if used on a non-prod network ###
##########################################################################
VOTING_PERIOD_DAYS = 4
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: we should just move this under Required proposal fields. for simplicity

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this is fine for now


##########################################################################
### Script contants - do not change ###
@@ -51,50 +63,45 @@
DELAY_MSG_MODULE_ADDRESS="dydx1mkkvp26dngu6n8rmalaxyp3gwkjuzztq5zx6tr"
DEPOSIT=f"10000{NINE_ZEROS}{NINE_ZEROS}{NATIVE_TOKEN_DENOM}" # 10,000 native tokens
OUTPUT_FILE='proposal_bridge_vesters_set_rewards.json'
REF_BLOCK_HEIGHT_FOR_ESTIMATE=1793496
REF_BLOCK_TIME_FOR_ESTIMATE="2023-11-23T17:10:19+00:00"

# Helper function to estimate future block time.
def estimate_block_height(current_block_height, current_block_timestamp, average_block_time, future_timestamp):
"""
Estimates the block height at a future timestamp.

:param current_block_height: Current height of the blockchain (in blocks)
:param current_block_timestamp: UTC Timestamp of the current block (ISO 8601 format)
:param average_block_time: Average time it takes to mine a block (in seconds, can be a float)
:param future_timestamp: UTC Future timestamp for which block height is to be estimated (ISO 8601 format)
:return: Estimated block height at the future timestamp
"""

# Helper function to estimate number of blocks between two times.
def estimate_blocks_between_timestamps(base_block_timestamp, average_block_time, future_timestamp):
# Convert ISO 8601 timestamps to datetime objects
current_block_time = parser.isoparse(current_block_timestamp)
base_block_time = parser.isoparse(base_block_timestamp)
future_time = parser.isoparse(future_timestamp)

# Calculate the time difference in seconds
time_difference = (future_time - current_block_time).total_seconds()
time_difference = (future_time - base_block_time).total_seconds()
Copy link
Contributor

Choose a reason for hiding this comment

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

should we add assertion that future - base is not negative?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is effectively checked below with

if delay_blocks_update_1 <= 0 or delay_blocks_update_2 <= 0:
    sys.exit(f"Estimated delay_blocks <= 0: {delay_blocks_update_1}, {delay_blocks_update_2}")


# Estimate the number of blocks that will be added in this time
estimated_blocks = float(time_difference) / average_block_time

# Calculate the estimated future block height
estimated_future_block_height = current_block_height + estimated_blocks
return int(round(estimated_blocks))

return int(round(estimated_future_block_height))
# Get current time in UTC
current_utc_time = datetime.now(pytz.utc)
# Add voting period so we can use the estimated time for gov proposal execution.
estimated_proposal_pass_time = current_utc_time + timedelta(days=VOTING_PERIOD_DAYS)
formatted_estimated_proposal_pass_time = estimated_proposal_pass_time.strftime("%Y-%m-%dT%H:%M:%S+00:00")

rewards_update_1_block = estimate_block_height(
REF_BLOCK_HEIGHT_FOR_ESTIMATE,
REF_BLOCK_TIME_FOR_ESTIMATE,
# Delayed update block heights are expected be after the proposal pass time.
delay_blocks_update_1 = estimate_blocks_between_timestamps(
formatted_estimated_proposal_pass_time,
AVG_BLOCK_TIME_FOR_ESTIMATE,
UPDATE_1_TIME_UTC,
)
rewards_update_2_block = estimate_block_height(
REF_BLOCK_HEIGHT_FOR_ESTIMATE,
REF_BLOCK_TIME_FOR_ESTIMATE,
delay_blocks_update_2 = estimate_blocks_between_timestamps(
formatted_estimated_proposal_pass_time,
AVG_BLOCK_TIME_FOR_ESTIMATE,
UPDATE_2_TIME_UTC,
)

print(f"Estimated block height for rewards multiplier update 1 {UPDATE_1_TIME_UTC} = {rewards_update_1_block}")
print(f"Estimated block height for rewards multiplier update 2 {UPDATE_2_TIME_UTC} = {rewards_update_2_block}")
if delay_blocks_update_1 <= 0 or delay_blocks_update_2 <= 0:
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
if delay_blocks_update_1 <= 0 or delay_blocks_update_2 <= 0:
if delay_blocks_update_1 < 0 or delay_blocks_update_2 < 0:

Copy link
Contributor Author

@teddyding teddyding Nov 24, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I think we should reject 0, since the delay_blocks should be positive to indicate a future block

sys.exit(f"Estimated delay_blocks <= 0: {delay_blocks_update_1}, {delay_blocks_update_2}")
Comment on lines +99 to +100
Copy link
Contributor

Choose a reason for hiding this comment

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

The script exits if the calculated delay in blocks for either update is less than or equal to zero. This is a good check, but consider also logging the current block height for better debugging information in case of an error.


print(f"Estimated proposal pass time ({VOTING_PERIOD_DAYS} days from now) = {formatted_estimated_proposal_pass_time}")
print(f"Estimated block delay for **first** update @ {UPDATE_1_TIME_UTC} = {delay_blocks_update_1}")
print(f"Estimated block delay for **second** update @ {UPDATE_2_TIME_UTC} = {delay_blocks_update_2}")

proposal_template = {
"title": TITLE,
@@ -125,10 +132,10 @@ def estimate_block_height(current_block_height, current_block_timestamp, average
"@type": "/dydxprotocol.rewards.MsgUpdateParams",
"authority": GOV_MODULE_ADDRESS,
"params": {
"treasuryAccount": "rewards_treasury",
Copy link
Contributor Author

@teddyding teddyding Nov 24, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

These are technically fine, since compiled params.pb.go looks like this

// Params defines the parameters for x/rewards module.
type Params struct {
	// The module account to distribute rewards from.
	TreasuryAccount string `protobuf:"bytes,1,opt,name=treasury_account,json=treasuryAccount,proto3" json:"treasury_account,omitempty"`
	// The denom of the rewards token.
	Denom string `protobuf:"bytes,2,opt,name=denom,proto3" json:"denom,omitempty"`
	// The exponent of converting one unit of `denom` to a full coin.
	// For example, `denom=uatom, denom_exponent=-6` defines that
	// `1 uatom = 10^(-6) ATOM`. This conversion is needed since the
	// `market_id` retrieves the price of a full coin of the reward token.
	DenomExponent int32 `protobuf:"zigzag32,3,opt,name=denom_exponent,json=denomExponent,proto3" json:"denom_exponent,omitempty"`
	// The id of the market that has the price of the rewards token.
	MarketId uint32 `protobuf:"varint,4,opt,name=market_id,json=marketId,proto3" json:"market_id,omitempty"`
	// The amount (in ppm) that fees are multiplied by to get
	// the maximum rewards amount.
	FeeMultiplierPpm uint32 `protobuf:"varint,5,opt,name=fee_multiplier_ppm,json=feeMultiplierPpm,proto3" json:"fee_multiplier_ppm,omitempty"`
}

Still updated for less confusion

"treasury_account": "rewards_treasury",
"denom": NATIVE_TOKEN_DENOM,
"denomExponent": -18,
"marketId": 1000001,
"denom_exponent": -18,
"market_id": 1000001,
"fee_multiplier_ppm": REWARDS_MULTIPLIER
}
},
@@ -137,8 +144,8 @@ def estimate_block_height(current_block_height, current_block_timestamp, average

# Add delayed messages
for delayed_block_number, new_fee_multiplier in [
(rewards_update_1_block, REWARDS_MULTIPLIER_UPDATE_1),
(rewards_update_2_block, REWARDS_MULTIPLIER_UPDATE_2),
(delay_blocks_update_1, REWARDS_MULTIPLIER_UPDATE_1),
(delay_blocks_update_2, REWARDS_MULTIPLIER_UPDATE_2),
]:
proposal_template["messages"].append(
{
@@ -148,10 +155,10 @@ def estimate_block_height(current_block_height, current_block_timestamp, average
"@type": "/dydxprotocol.rewards.MsgUpdateParams",
"authority": DELAY_MSG_MODULE_ADDRESS,
"params": {
"treasuryAccount": "rewards_treasury",
"treasury_account": "rewards_treasury",
"denom": NATIVE_TOKEN_DENOM,
"denomExponent": -18,
"marketId": 1000001,
"denom_exponent": -18,
"market_id": 1000001,
"fee_multiplier_ppm": new_fee_multiplier
}
},