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

VIP: native asset types #3716

Open
charles-cooper opened this issue Dec 29, 2023 · 16 comments
Open

VIP: native asset types #3716

charles-cooper opened this issue Dec 29, 2023 · 16 comments
Milestone

Comments

@charles-cooper
Copy link
Member

charles-cooper commented Dec 29, 2023

Simple Summary

bring new types to vyper which safely model assets, natively

Motivation

accounting is hard. instead of assets being represented by raw numbers as they currently are, add a special kind of asset type which is more constrained. this removes the possibility for rounding errors and will increase clarity of user code.

this new type enforces the invariant that each action to the ledger has an equal and opposite reaction. in code- terms, it un-denormalizes code, increases DRYness and reduces the potential for accounting bugs (ex. rounding errors, missed actions).

# example buggy code
def mint10():
    self.balances[addr] += 10
    self.totalSupply += 19  # typo
# with assets
def mint10():
    self.balances[addr] += 10   # error! can't assign directly
    self.balances[addr] = 10   # error! can't assign directly

    self.balances.move_from(self.totalSupply, 10)

notes:

  • I previously proposed a language feature in VIP: [research] Static balance sheet analysis for Vyper contracts #1277 to solve this problem, but the form was somewhat clunky and depended on an SMT solver to work.
  • @jacqueswww proposed a similar function in VIP: [research] Static balance sheet analysis for Vyper contracts #1277 (comment), this proposal improves on it by adding a native type with appropriate constraints for the user
  • other languages including flintlang and move propose mechanisms involving linear types, but i don't think applying the borrow checker model to assets really models it correctly. the purpose of linear / affine types is to ensure resources are used exactly / at most once. this is useful for RAII use cases. but the fundamental point here is that assets are never created or destroyed, only transferred between ledger tables. so applying the linear type model is a bit like trying to fit a round peg in a square hole.

Specification

add a new parametrizable type, Asset to vyper. Asset takes two type parameters, the subtype and an "intrinsic sign", which basically just corresponds to whether the account is debit- or credit- normal.

Asset cannot be assigned to directly, but can only be modified through the builtin move_from() method. (if you squint closely, move_from() is basically a single, balancing debit+credit).

example:

totalSupply: Asset[uint256, -]  # move_from totalSupply *increases* totalSupply
balances: HashMap[address, Asset[uint256, +]  # move_from a balance *decreases* the balance

def transfer(recipient: addr, amount: uint256):
    self.balances[recipient].move_from(self.balances[msg.sender], amount)

def mint(recipient: addr, amount: uint256):
    self.balances[recipient].move_from(self.totalSupply, amount)

tbd:

  • open to other names for Asset
  • the +/- syntax might look a little funky to programmers, maybe Asset vs NegativeAsset are more intuitive
  • move_from could be unintuitive when the source and destination accounts have opposite intrinsic signs. if this is the case, maybe additional methods like mint_to(src: Asset, dst: NegativeAsset) and burn_from(src: Asset, dst: NegativeAsset) could potentially be considered. the usage of move_from in the above mint() function would not be allowed and it would instead be written as
    def mint(...):
        self.balances[recipient].mint_from(self.totalSupply, amount)

Backwards Compatibility

no breaking changes

References

#1277
https://en.wikipedia.org/wiki/Debits_and_credits

Copyright

Copyright and related rights waived via CC0

@pcaversaccio
Copy link
Collaborator

Overall I like this idea very much. But I have to admit that I had to read the specs multiple times to understand the semantics of the "intrinsic sign" correctly. I think the word move can also be debated. Like asset creations (i.e. what you refer to minting above) could be called create_resource(...) and destroying/removing assets could be simply called destroy_resource(...) (see below my naming suggestion). For moving the asset, it could be simply transfer_resource(...). This wording is much faster to comprehend. Also, what do you think about this syntax instead (I implicitly want to kick off the convo around generics):

T = vyper.TypeVar("uint256")
totalSupply: public(Resource[T])
balanceOf: public(HashMap[address, Resource[vyper.type(self.totalSupply)])

def transfer(to: address, amount: uint256):
    transfer_resource(self.totalSupply, self.balanceOf[msg.sender], self.balanceOf[to], amount)

def mint(owner: address, amount: uint256):
    create_resource(self.totalSupply, empty(address), self.balanceOf[owner], amount)

def burn(owner: address, amount: uint256):
    destroy_resource(self.totalSupply, self.balanceOf[owner], empty(address), amount)

So the functions would be like:

transfer_resource(resource: Resource[T], resource_origin: HashMap[address, Resource[vyper.type(resource)]], resource_destination: HashMap[address, Resource[vyper.type(resource)]], resource_amount: uint256)
create_resource(resource: Resource[T], resource_origin: address=empty(address), resource_destination: HashMap[address, Resource[vyper.type(resource)]], resource_amount: uint256)
destroy_resource(resource: Resource[T], resource_origin: HashMap[address, Resource[vyper.type(resource)]], resource_destination: address=empty(address), resource_amount: uint256)

We might want to have an unsafe version of it for people who wanna skip the compiler invariant checks to save gas and assume they know what they do :). Maybe this can be implemented via a kwarg...

I personally like Resource as a name since it's somehow more general. Asset is very finger-pointing to DeFi somehow, which is fine, but we should consider a name that is more generic IMO.

@charles-cooper
Copy link
Member Author

i have a slight preference for "asset"- related terminology. "resource" sounds more like filehandles or linear types.

i think maybe the key insight from the "theory" of double-entry accounting being applied here is that assets are never created or destroyed, only moved -- and the way it is able to work is because some accounts have opposite intrinsic sign than others. so balances[addr].move_from(totalSupply) is a debit to totalSupply and a credit to balances, but that increases the value of both accounts. it enforces the invariant totalSupply - sum(balances) == 0 by construction!

@charles-cooper
Copy link
Member Author

recommended reading for those unfamiliar with the debits/credits terminology: https://en.wikipedia.org/wiki/Debits_and_credits

@fubuloubu

This comment was marked as off-topic.

@charles-cooper

This comment was marked as off-topic.

@pcaversaccio

This comment was marked as off-topic.

@pcaversaccio

This comment was marked as off-topic.

@fubuloubu

This comment was marked as off-topic.

@charles-cooper

This comment was marked as off-topic.

@charles-cooper

This comment was marked as off-topic.

@fubuloubu

This comment was marked as off-topic.

@charles-cooper

This comment was marked as off-topic.

@fubuloubu
Copy link
Member

Direct feedback: the "intrinsic sign" is very hard to understand, and using operators seems quite likely to be overlooked when auditing. Would at least suggest using some sort of built-in enum relating to that new type e.g.: Asset[<type>, Asset.CREDIT] or something.

Further feel like it doesn't have to be a language-level built-in type with some more generic features made available, it could be implemented as a user-generated type

@charles-cooper
Copy link
Member Author

Direct feedback: the "intrinsic sign" is very hard to understand, and using operators seems quite likely to be overlooked when auditing. Would at least suggest using some sort of built-in enum relating to that new type e.g.: Asset[, Asset.CREDIT] or something.

yea, i agree that the intrinsic sign is not super intuitive as an API. i think a better API is to have two separate types Asset and DAsset, and instead of a single move_from (which allows mixing between the two types of asset), segregate into three functions similar to @pcaversaccio 's suggestion:

transfer_from(dst: Asset[T], src: Asset[T], T)
mint_from(dst: Asset[T], src: DAsset[T], T)
burn_from(dst: DAsset[T], src: Asset[T], T)

these all do the same thing(!), debit src and credit dst, but it's probably a more intuitive API for most programmers and also a little more type-safe.

Further feel like it doesn't have to be a language-level built-in type with some more generic features made available, it could be implemented as a user-generated type

it would be neat if it could be implemented with pure vyper :). but even if it can't, i don't think that should be a blocker for inclusion in the language. safe accounting is important enough to smart contract programming that i think it should have first-class support in a smart contract language!

if at a later date vyper does support generics (with the necessary intrinsics/protocols), it could maybe be reimplemented in pure vyper as part of the standard library, but i don't think we need to block the feature waiting on generics.

@charles-cooper charles-cooper changed the title VIP [draft]: native asset types VIP: native asset types Jan 15, 2024
@charles-cooper charles-cooper added this to the v0.4.2 milestone Oct 20, 2024
@Philogy
Copy link
Contributor

Philogy commented Nov 3, 2024

Wanted to add my off-github comment here: Good idea but seems very limited in use case. Also forces you to have a storage variable for every total which may not be desirable e.g. if you want to use this to track token deposits you now need an Asset[uint256, -] total variable to track the counter total part.

@fubuloubu
Copy link
Member

fubuloubu commented Nov 3, 2024

Wanted to stick a comment from the other day in here, but I think the best use cases for this type of thing is managing external assets (e.g. ERC4626), not ERC20 ledger values. So like, associating a storage variable with a particular asset type in such a way that they don't get out of sync (a common source of bugs like inflation attacks, improper asset controls, etc.)


One nice thing might even to be able say like self.token.balanceOf(self.address) - self.token.total(), which would give you the "untracked" amount of balance that the contract has (from an airdrop or accidental transfer). Maybe can do something like self.token.move(self.address, receiver, amount) that simultaneously does ERC20 transfer from this contract out to receiver and also subtracts from an internal ledger bookkeeping that amount

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

No branches or pull requests

5 participants
@fubuloubu @charles-cooper @Philogy @pcaversaccio and others