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

Jump to bottom

Reinvent lockup #409

Merged
merged 26 commits into from
Jan 9, 2019
Merged

Reinvent lockup #409

merged 26 commits into from
Jan 9, 2019

Conversation

satyamakgec
Copy link
Contributor

@satyamakgec satyamakgec commented Nov 13, 2018 •
edited by VictorVicente
Loading

  • Test suite

@satyamakgec satyamakgec changed the title [WIP] Reinvent lockup Reinvent lockup Nov 14, 2018
@satyamakgec satyamakgec assigned adamdossa and maxsam4 and unassigned adamdossa and maxsam4 Nov 14, 2018
@satyamakgec
Copy link
Contributor Author

Please do check with the issue #389 As per the test it is not present in the current code

This was referenced Nov 15, 2018
Closed
Closed
@satyamakgec satyamakgec mentioned this pull request Nov 15, 2018
Closed
maxsam4
maxsam4 suggested changes Nov 16, 2018
View reviewed changes
Copy link
Contributor

@maxsam4 maxsam4 left a comment

Choose a reason for hiding this comment

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

Consider adding a view getLockedToken function which will return sum of cirrently locked tokens of a user so that user can know how many tokens are still locked and how many can they transfer.

contracts/modules/Experimental/TransferManager/LockupVolumeRestrictionTM.sol Outdated Show resolved Hide resolved
contracts/modules/Experimental/TransferManager/LockupVolumeRestrictionTM.sol Outdated Show resolved Hide resolved
contracts/modules/Experimental/TransferManager/LockupVolumeRestrictionTM.sol Outdated Show resolved Hide resolved
contracts/modules/Experimental/TransferManager/LockupVolumeRestrictionTM.sol Outdated Show resolved Hide resolved
contracts/modules/Experimental/TransferManager/LockupVolumeRestrictionTM.sol Outdated Show resolved Hide resolved
contracts/modules/Experimental/TransferManager/LockupVolumeRestrictionTM.sol Outdated Show resolved Hide resolved
contracts/modules/Experimental/TransferManager/LockupVolumeRestrictionTM.sol Outdated Show resolved Hide resolved
@maxsam4
Copy link
Contributor

maxsam4 commented Nov 16, 2018

TIL that references to parameters of an external function are stored in the stack.

@satyamakgec
Copy link
Contributor Author

@maxsam4 It is done with the review? If yes then @VictorVicente can use this same PR to create the CLI

@maxsam4
Copy link
Contributor

maxsam4 commented Nov 20, 2018

Yeah. The function modifier thing was the last issue.
Everything Looks good to me now.

@satyamakgec
Copy link
Contributor Author

@maxsam4 @adamdossa could you please review it again because of the data structure changes

@coveralls
Copy link

coveralls commented Jan 8, 2019 •
edited
Loading

Coverage Status

Coverage decreased (-0.3%) to 96.042% when pulling 887860e on reinvent-lockup into 7e05021 on dev-2.1.0.

@VictorVicente VictorVicente merged commit 70a460a into dev-2.1.0 Jan 9, 2019
@VictorVicente VictorVicente deleted the reinvent-lockup branch January 9, 2019 15:30
@knanjukutty-polymath knanjukutty-polymath mentioned this pull request Mar 28, 2024
Open
@knanjukutty-polymath knanjukutty-polymath mentioned this pull request Apr 17, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maxsam4 maxsam4 maxsam4 approved these changes

@adamdossa adamdossa adamdossa approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@satyamakgec @maxsam4 @coveralls @adamdossa @VictorVicente @SatyamSB