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

Introduce the Pausable Library #199

Closed
wants to merge 10 commits into from
Closed

Introduce the Pausable Library #199

wants to merge 10 commits into from

Conversation

bitzoic
Copy link
Member

@bitzoic bitzoic commented Oct 23, 2023

Type of change

  • New Library

Changes

The following changes have been made:

  • Adds the Pausable Library to the sway-libs repo

Notes

  • This library is completely stateless and relies on a pre-defined sub-id to mint and burn an asset that represents a state. This is being called "packet-oriented design" and saves users 20% gas in my tests as opposed to storing a bool in storage. It also does not require that the developer pass a StorageKey for the library to properly function.

Related Issues

Closes #198

@bitzoic bitzoic added Enhancement New feature or request Lib: Security Label used to filter for the library issue labels Oct 23, 2023
@bitzoic bitzoic self-assigned this Oct 23, 2023
@bitzoic bitzoic requested a review from a team as a code owner October 23, 2023 09:35
@bitzoic bitzoic linked an issue Oct 23, 2023 that may be closed by this pull request
Copy link
Contributor

@Braqzen Braqzen left a comment

Choose a reason for hiding this comment

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

Outside of global search and replace knits in this whole PR there's only 1 point to make.

This is an interesting design mechanism but its design is flawed. There is a comment to use admin restrictions nevertheless it allows for spamming of an asset if somewhere where to go wrong and have the balance altered. This may affect the market.

Moreover, it's really a hack to save costs rather than a solid design. I don't believe in promoting hacks. If this wasn't a security based library then I'd have less of an issue with a hack and some asset potentially being nuked in value so I'd go back to statefulness

libs/pausable/src/lib.sw Outdated Show resolved Hide resolved
libs/pausable/src/lib.sw Outdated Show resolved Hide resolved
libs/pausable/README.md Outdated Show resolved Hide resolved
@bitzoic
Copy link
Member Author

bitzoic commented Oct 24, 2023

This is an interesting design mechanism but its design is flawed. There is a comment to use admin restrictions nevertheless it allows for spamming of an asset if somewhere where to go wrong and have the balance altered. This may affect the market.

Your comment makes no sense. The design is not flawed. Admin restrictions only allow a SINGLE user to pause rather than anyone. There cannot be spamming of an asset, when minting may only happen in the case of NO tokens. There is NO market involved.

Moreover, it's really a hack to save costs rather than a solid design. I don't believe in promoting hacks. If this wasn't a security based library then I'd have less of an issue with a hack and some asset potentially being nuked in value so I'd go back to statefulness

This is NOT a hack, it is leveraging UTXO design. This was discussed a number of times as packet packet-oriented design with the use of Sub Ids. Again, there is NO value.

@Braqzen
Copy link
Contributor

Braqzen commented Oct 24, 2023

This is an interesting design mechanism but its design is flawed. There is a comment to use admin restrictions nevertheless it allows for spamming of an asset if somewhere where to go wrong and have the balance altered. This may affect the market.

Your comment makes no sense. The design is not flawed. Admin restrictions only allow a SINGLE user to pause rather than anyone. There cannot be spamming of an asset, when minting may only happen in the case of NO tokens. There is NO market involved.

Moreover, it's really a hack to save costs rather than a solid design. I don't believe in promoting hacks. If this wasn't a security based library then I'd have less of an issue with a hack and some asset potentially being nuked in value so I'd go back to statefulness

This is NOT a hack, it is leveraging UTXO design. This was discussed a number of times as packet packet-oriented design with the use of Sub Ids. Again, there is NO value.

Pause -> mint -> somehow withdraw the balance -> in "paused" but actually unpaused state. Design flaw.
Again, you're wrong. It is a hack. You're minting outside the purpose of a mint. If you could create and destroy something that wouldn't have any value, equivalent to a bool, that would be fine.

@bitzoic
Copy link
Member Author

bitzoic commented Oct 24, 2023

This is an interesting design mechanism but its design is flawed. There is a comment to use admin restrictions nevertheless it allows for spamming of an asset if somewhere where to go wrong and have the balance altered. This may affect the market.

Your comment makes no sense. The design is not flawed. Admin restrictions only allow a SINGLE user to pause rather than anyone. There cannot be spamming of an asset, when minting may only happen in the case of NO tokens. There is NO market involved.

Moreover, it's really a hack to save costs rather than a solid design. I don't believe in promoting hacks. If this wasn't a security based library then I'd have less of an issue with a hack and some asset potentially being nuked in value so I'd go back to statefulness

This is NOT a hack, it is leveraging UTXO design. This was discussed a number of times as packet packet-oriented design with the use of Sub Ids. Again, there is NO value.

Pause -> mint -> somehow withdraw the balance -> in "paused" but actually unpaused state. Design flaw. Again, you're wrong. It is a hack. You're minting outside the purpose of a mint. If you could create and destroy something that wouldn't have any value, equivalent to a bool, that would be fine.

"somehow withdraw the balance" is an impossible hypothetical situation considering the contract owns the minted asset. Again, the asset has no value.

@Braqzen
Copy link
Contributor

Braqzen commented Oct 24, 2023

This is an interesting design mechanism but its design is flawed. There is a comment to use admin restrictions nevertheless it allows for spamming of an asset if somewhere where to go wrong and have the balance altered. This may affect the market.

Your comment makes no sense. The design is not flawed. Admin restrictions only allow a SINGLE user to pause rather than anyone. There cannot be spamming of an asset, when minting may only happen in the case of NO tokens. There is NO market involved.

Moreover, it's really a hack to save costs rather than a solid design. I don't believe in promoting hacks. If this wasn't a security based library then I'd have less of an issue with a hack and some asset potentially being nuked in value so I'd go back to statefulness

This is NOT a hack, it is leveraging UTXO design. This was discussed a number of times as packet packet-oriented design with the use of Sub Ids. Again, there is NO value.

Pause -> mint -> somehow withdraw the balance -> in "paused" but actually unpaused state. Design flaw. Again, you're wrong. It is a hack. You're minting outside the purpose of a mint. If you could create and destroy something that wouldn't have any value, equivalent to a bool, that would be fine.

"somehow withdraw the balance" is an impossible hypothetical situation considering the contract owns the minted asset. Again, the asset has no value.

I add the ability to withdraw some asset in some way and get my hands on your specific asset by some exploit.
"Don't do that" is not counter argument. "Just make an unhackable bridge", "Just don't get hacked".

@bitzoic
Copy link
Member Author

bitzoic commented Oct 27, 2023

Closed in favor of #200

bitzoic added a commit that referenced this pull request Oct 30, 2023
## Type of change

<!--Delete points that do not apply-->

- New Library

## Changes

The following changes have been made:

- Introduces the pausable library to the repo

## Notes

- The original design used a packet-oriented approach. However due to
changes in gas outlined
[here](FuelLabs/fuel-core#1408 (comment)),
a storage approach has instead been selected as mint/burn opcodes have
become significantly more expensive.

## Related Issues

<!--Delete everything after the "#" symbol and replace it with a number.
No spaces between hash and number-->

Closes [#199 ](#198)

---------

Co-authored-by: bitzoic <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request Lib: Security Label used to filter for the library issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pausable Library
2 participants