-
Notifications
You must be signed in to change notification settings - Fork 1
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
refactor: Launchpad Architecture Refactoring #451
base: main
Are you sure you want to change the base?
Conversation
return totalAmount, nil | ||
} | ||
|
||
func CollectDepositGns() uint64 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't we need to check if the launchpad program is finished or not?
Deposited GNS is only claimable when the longest tier in a launchpad program is finished. (project token rewards are always claimable regardless of this)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that condition checks has been removed in somewhere in the commits. But I can't find where it was.
launchpad/deposit.gno
Outdated
// depositId -> deposit | ||
deposits = make(map[string]Deposit) | ||
|
||
// proejct -> tier -> []depositId | ||
depositsByProject = make(map[string]map[string][]string) | ||
|
||
// user -> []depositId | ||
depositsByUser = make(map[std.Address][]string) | ||
|
||
// user -> project -> []depositId | ||
depositsByUserByProject = make(map[std.Address]map[string][]string) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
avl
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The launchpad's map type wasn't modified to AVL since the values used inside the existing map type are primitive types. (This answer applies to all similar comments below)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@onlyhyde Are we keeping primitive maps?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@r3v4s
Yes,
Although our policy was to use avl.Tree all the time,
In my opinion, launchpad doesn't use pointer variables, which is a problem with maps, so I don't think it's necessary to change everything to avl.Tree, since using maps saves gas and doesn't cause the problem with maps.
Deposits map[string]Deposit | ||
DepositsByProject map[string]map[string][]string | ||
DepositsByUser map[std.Address][]string | ||
DepositsByUserProject map[std.Address]map[string][]string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
avl
@@ -30,7 +30,7 @@ func TestCreateProjectSingleRecipient(t *testing.T) { | |||
testCreateProject(t) | |||
testMockProtocolFee(t) | |||
testDepositGnsToTier30(t) | |||
testCollectProtocolFee(t) | |||
// testCollectProtocolFee(t) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reason for commenting?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left few comments.
- FYI, I'm aware of feat: add reward cacheing logic to launchpad and governance #455 going on.
- If there are plan to fix things in feat: add reward cacheing logic to launchpad and governance #455, reply it in conversation please.
) | ||
|
||
func currentBalance() uint64 { | ||
func getCurrentBalance() uint64 { | ||
// TODO: implement this after checking gns distribution is working |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
handle TODO
@@ -32,11 +33,27 @@ func currentBalance() uint64 { | |||
return currentGNSBalance | |||
} | |||
|
|||
func getCurrentProtocolFeeBalance() map[string]uint64 { | |||
gotAccuProtocolFee := pf.GetAccuTransferToGovStaker() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe missing pf.DistributeProtocolFee()
??
or remove this function and uncomment L#23
Quality Gate passedIssues Measures |
Description
This PR introduces a comprehensive refactoring of the Launchpad contract architecture, focusing on optimizing reward calculations and improving data structure efficiency. The main goals are to reduce computational overhead, enhance gas efficiency, and improve the overall maintainability of the codebase.
Key Changes
Architectural Improvements
ProjectInput
,ProjectTierInfo
,DepositState
)Code Structure
Major Refactored Components
Project Management (
launchpad_init.gno
->launchpad.gno
)validateProjectInput
createTier
Reward Handling (
launchpad_reward.gno
->reward.gno
)Deposit Management (
launchpad_deposit.gno
)DepositState
for better state managementvalidateDepositCollection
)Simplified and More Organized Types
TimeInfo
: Consolidated height and timestamp pairsProjectStats
: Grouped project statistics fieldsRefundInfo
: Consolidated refund-related fieldsProject Structure Improvements
Project
struct for better maintainability:map[uint64]Tier
)tiersRatios
map for cleaner ratio managementTimeInfo
ProjectStats
RefundInfo
Benefits of New Structure
Memory Optimization
Performance Considerations
Future Improvements