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

[#256] Allow more xtz entrypoints #269

Merged
merged 1 commit into from
May 26, 2021

Conversation

rinn7e
Copy link
Contributor

@rinn7e rinn7e commented May 26, 2021

Description

Problem: We currently have a safety check for most entrypoints to
disallow xtz tokens to be mistakenly sent to the contract and allow
these funds to be stuck there.
We need to reconsider on which entrypoints we should keep this check
and on which we don't need to (see #91).

Solution: Move accept_ownership and transfer_ownership to
allowing_xtz entrypoints.

Related issue(s)

Resolves #256

✅ Checklist for your Pull Request

Related changes (conditional)

  • Tests
    • If I added new functionality, I added tests covering it.
    • If I fixed a bug, I added a regression test to prevent the bug from
      silently reappearing again.
  • Documentation
    • I checked whether I should update the docs and did so if necessary:

Stylistic guide (mandatory)

@rinn7e rinn7e requested review from pasqu4le and sras May 26, 2021 12:22
Copy link
Contributor

@pasqu4le pasqu4le left a comment

Choose a reason for hiding this comment

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

LGTM, my only nitpick is that I would replace Add with Move in the commit message Solution.

Problem: We currently have a safety check for most entrypoints to
disallow xtz tokens to be mistakenly sent to the contract and allow
these funds to be stuck there.
We need to reconsider on which entrypoints we should keep this check
and on which we don't need to (see #91).

Solution: Move `accept_ownership` and `transfer_ownership` to
`allowing_xtz` entrypoints.
@rinn7e rinn7e force-pushed the rinn7e/#256-update-tez-check-entrypoints branch from 6a2cad8 to b499638 Compare May 26, 2021 13:16
@rinn7e rinn7e merged commit fbd85af into master May 26, 2021
@rinn7e rinn7e deleted the rinn7e/#256-update-tez-check-entrypoints branch May 26, 2021 13:46
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.

Modify entrypoints on which to apply the no-xtz check
2 participants