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

Made some TokenVesting public functions private. #1427

Merged
merged 3 commits into from
Oct 17, 2018

Conversation

nventuro
Copy link
Contributor

VestedAmount can be manipulated by transferring tokens, so while the contract works properly in all cases, it makes no sense to expose this intermediate value.

@nventuro nventuro added contracts Smart contract code. breaking change Changes that break backwards compatibility of the public API. labels Oct 17, 2018
@nventuro nventuro added this to the v2.0 milestone Oct 17, 2018
@nventuro nventuro requested a review from come-maiz October 17, 2018 16:15
Copy link
Contributor

@come-maiz come-maiz left a comment

Choose a reason for hiding this comment

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

Um, ok. I have the feeling that we need to think more about this, because it feels weird not to be able to query the current state of the contract.
However, this is in drafts :) I'm happy to think some other time in the future. This makes sense for now.

@nventuro
Copy link
Contributor Author

I agree with your concern, but this fixed version works properly, and is a draft anyway. We'll probably do a full redesign of the vesting contract, since some schemes are not supported by it (see #1214).

@nventuro nventuro closed this Oct 17, 2018
@nventuro nventuro reopened this Oct 17, 2018
@nventuro nventuro merged commit df3c113 into OpenZeppelin:master Oct 17, 2018
@nventuro nventuro deleted the vesting-pub-fns branch October 17, 2018 21:18
come-maiz pushed a commit that referenced this pull request Oct 21, 2018
* Made some TokenVesting public functions private.

* Fixed linter error.

(cherry picked from commit df3c113)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change Changes that break backwards compatibility of the public API. contracts Smart contract code.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants