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

feat[ux]: improve hint for events kwarg upgrade #4275

Open
wants to merge 23 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
c4cc839
feat[ux]: improve hint for events kwarg upgrade
charles-cooper Oct 5, 2024
106bbfa
Merge branch 'master' into feat/improve-event-kwargs-recommendation
charles-cooper Oct 17, 2024
cf949f5
update warning
charles-cooper Nov 18, 2024
2044e94
update some examples
charles-cooper Nov 18, 2024
85fec8e
fix stray whitespace
charles-cooper Nov 18, 2024
3c98587
Merge branch 'master' into feat/improve-event-kwargs-recommendation
charles-cooper Nov 21, 2024
6550ac6
add tests for event kwargs hint
cyberthirst Dec 6, 2024
fe34000
Merge pull request #52 from cyberthirst/fork/charles-cooper/feat/impr…
charles-cooper Dec 6, 2024
def7bcb
Merge branch 'master' into feat/improve-event-kwargs-recommendation
charles-cooper Jan 1, 2025
5eae19a
Merge branch 'master' into feat/improve-event-kwargs-recommendation
charles-cooper Jan 9, 2025
483baf7
Merge branch 'master' into feat/improve-event-kwargs-recommendation
charles-cooper Jan 12, 2025
9e18307
tmp rollback
charles-cooper Jan 12, 2025
40c6c41
better recommendation
charles-cooper Jan 12, 2025
54200c7
update ERC20.vy
charles-cooper Jan 12, 2025
d4dd842
update ERC721.vy
charles-cooper Jan 12, 2025
d42ae0d
update ERC1155Ownable.vy
charles-cooper Jan 12, 2025
d591d08
update ERC4626.vy
charles-cooper Jan 12, 2025
8681e71
update blind_auction.vy
charles-cooper Jan 12, 2025
aa7c7bd
update advanced_storage.vy
charles-cooper Jan 12, 2025
9408dcf
update company.vy
charles-cooper Jan 12, 2025
fd15d4b
update tests
charles-cooper Jan 12, 2025
9c48102
Merge branch 'master' into feat/improve-event-kwargs-recommendation
charles-cooper Jan 20, 2025
b42801e
Merge branch 'master' into feat/improve-event-kwargs-recommendation
charles-cooper Jan 20, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion examples/auctions/blind_auction.vy
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ def auctionEnd():
assert not self.ended

# Log auction ending and set flag
log AuctionEnded(self.highestBidder, self.highestBid)
log AuctionEnded(highestBidder=self.highestBidder, highestBid=self.highestBid)
self.ended = True

# Transfer funds to beneficiary
Expand Down
9 changes: 5 additions & 4 deletions examples/stock/company.vy
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ def buyStock():
self.holdings[msg.sender] += buy_order

# Log the buy event.
log Buy(msg.sender, buy_order)
log Buy(buyer=msg.sender, buy_order=buy_order)

# Public function to allow external access to _getHolding
@view
Expand Down Expand Up @@ -94,7 +94,7 @@ def sellStock(sell_order: uint256):
send(msg.sender, sell_order * self.price)

# Log the sell event.
log Sell(msg.sender, sell_order)
log Sell(seller=msg.sender, sell_order=sell_order)

# Transfer stock from one stockholder to another. (Assume that the
# receiver is given some compensation, but this is not enforced.)
Expand All @@ -109,7 +109,7 @@ def transferStock(receiver: address, transfer_order: uint256):
self.holdings[receiver] += transfer_order

# Log the transfer event.
log Transfer(msg.sender, receiver, transfer_order)
log Transfer(sender=msg.sender, receiver=receiver, value=transfer_order)

# Allow the company to pay someone for services rendered.
@external
Expand All @@ -123,7 +123,8 @@ def payBill(vendor: address, amount: uint256):
send(vendor, amount)

# Log the payment event.
log Pay(vendor, amount)
log Pay(vendor=vendor, amount=amount)


# Public function to allow external access to _debt
@view
Expand Down
2 changes: 1 addition & 1 deletion examples/storage/advanced_storage.vy
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ def set(_x: int128):
assert _x >= 0, "No negative values"
assert self.storedData < 100, "Storage is locked when 100 or more is stored"
self.storedData = _x
log DataChange(msg.sender, _x)
log DataChange(setter=msg.sender, value=_x)

@external
def reset():
Expand Down
28 changes: 14 additions & 14 deletions examples/tokens/ERC1155ownable.vy
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ event unPaused:

event OwnershipTransferred:
# Emits smart contract ownership transfer from current to new owner
previouwOwner: address
previousOwner: address
newOwner: address

event TransferSingle:
Expand Down Expand Up @@ -150,7 +150,7 @@ def pause():
assert self.owner == msg.sender, "Ownable: caller is not the owner"
assert not self.paused, "the contract is already paused"
self.paused = True
log Paused(msg.sender)
log Paused(account=msg.sender)

@external
def unpause():
Expand All @@ -162,7 +162,7 @@ def unpause():
assert self.owner == msg.sender, "Ownable: caller is not the owner"
assert self.paused, "the contract is not paused"
self.paused = False
log unPaused(msg.sender)
log unPaused(account=msg.sender)

## ownership ##
@external
Expand All @@ -179,7 +179,7 @@ def transferOwnership(newOwner: address):
assert newOwner != empty(address), "Transfer to the zero address not allowed. Use renounceOwnership() instead."
oldOwner: address = self.owner
self.owner = newOwner
log OwnershipTransferred(oldOwner, newOwner)
log OwnershipTransferred(previousOwner=oldOwner, newOwner=newOwner)

@external
def renounceOwnership():
Expand All @@ -191,7 +191,7 @@ def renounceOwnership():
assert self.owner == msg.sender, "Ownable: caller is not the owner"
oldOwner: address = self.owner
self.owner = empty(address)
log OwnershipTransferred(oldOwner, empty(address))
log OwnershipTransferred(previousOwner=oldOwner, newOwner=empty(address))

@external
@view
Expand Down Expand Up @@ -226,7 +226,7 @@ def mint(receiver: address, id: uint256, amount:uint256):
assert receiver != empty(address), "Can not mint to ZERO ADDRESS"
operator: address = msg.sender
self.balanceOf[receiver][id] += amount
log TransferSingle(operator, empty(address), receiver, id, amount)
log TransferSingle(operator=operator, fromAddress=empty(address), to=receiver, id=id, value=amount)


@external
Expand All @@ -249,7 +249,7 @@ def mintBatch(receiver: address, ids: DynArray[uint256, BATCH_SIZE], amounts: Dy
break
self.balanceOf[receiver][ids[i]] += amounts[i]

log TransferBatch(operator, empty(address), receiver, ids, amounts)
log TransferBatch(operator=operator, fromAddress=empty(address), to=receiver, ids=ids, values=amounts)

## burn ##
@external
Expand All @@ -263,7 +263,7 @@ def burn(id: uint256, amount: uint256):
assert not self.paused, "The contract has been paused"
assert self.balanceOf[msg.sender][id] > 0 , "caller does not own this ID"
self.balanceOf[msg.sender][id] -= amount
log TransferSingle(msg.sender, msg.sender, empty(address), id, amount)
log TransferSingle(operator=msg.sender, fromAddress=msg.sender, to=empty(address), id=id, value=amount)

@external
def burnBatch(ids: DynArray[uint256, BATCH_SIZE], amounts: DynArray[uint256, BATCH_SIZE]):
Expand All @@ -283,7 +283,7 @@ def burnBatch(ids: DynArray[uint256, BATCH_SIZE], amounts: DynArray[uint256, BAT
break
self.balanceOf[msg.sender][ids[i]] -= amounts[i]

log TransferBatch(msg.sender, msg.sender, empty(address), ids, amounts)
log TransferBatch(operator=msg.sender, fromAddress=msg.sender, to=empty(address), ids=ids, values=amounts)

## approval ##
@external
Expand All @@ -298,7 +298,7 @@ def setApprovalForAll(owner: address, operator: address, approved: bool):
assert not self.paused, "The contract has been paused"
assert owner != operator, "ERC1155: setting approval status for self"
self.isApprovedForAll[owner][operator] = approved
log ApprovalForAll(owner, operator, approved)
log ApprovalForAll(account=owner, operator=operator, approved=approved)

@external
def safeTransferFrom(sender: address, receiver: address, id: uint256, amount: uint256, bytes: bytes32):
Expand All @@ -317,7 +317,7 @@ def safeTransferFrom(sender: address, receiver: address, id: uint256, amount: ui
operator: address = msg.sender
self.balanceOf[sender][id] -= amount
self.balanceOf[receiver][id] += amount
log TransferSingle(operator, sender, receiver, id, amount)
log TransferSingle(operator=operator, fromAddress=sender, to=receiver, id=id, value=amount)

@external
def safeBatchTransferFrom(sender: address, receiver: address, ids: DynArray[uint256, BATCH_SIZE], amounts: DynArray[uint256, BATCH_SIZE], _bytes: bytes32):
Expand All @@ -342,7 +342,7 @@ def safeBatchTransferFrom(sender: address, receiver: address, ids: DynArray[uint
self.balanceOf[sender][id] -= amount
self.balanceOf[receiver][id] += amount

log TransferBatch(operator, sender, receiver, ids, amounts)
log TransferBatch(operator=operator, fromAddress=sender, to=receiver, ids=ids, values=amounts)

# URI #
@external
Expand All @@ -355,7 +355,7 @@ def setURI(uri: String[MAX_URI_LENGTH]):
assert self.baseuri != uri, "new and current URI are identical"
assert msg.sender == self.owner, "Only the contract owner can update the URI"
self.baseuri = uri
log URI(uri, 0)
log URI(value=uri, id=0)

@external
def toggleDynUri(status: bool):
Expand Down Expand Up @@ -391,7 +391,7 @@ def setContractURI(contractUri: String[MAX_URI_LENGTH]):
assert self.contractURI != contractUri, "new and current URI are identical"
assert msg.sender == self.owner, "Only the contract owner can update the URI"
self.contractURI = contractUri
log URI(contractUri, 0)
log URI(value=contractUri, id=0)

@view
@external
Expand Down
13 changes: 6 additions & 7 deletions examples/tokens/ERC20.vy
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,7 @@ def __init__(_name: String[32], _symbol: String[32], _decimals: uint8, _supply:
self.balanceOf[msg.sender] = init_supply
self.totalSupply = init_supply
self.minter = msg.sender
log IERC20.Transfer(empty(address), msg.sender, init_supply)

log IERC20.Transfer(sender=empty(address), receiver=msg.sender, value=init_supply)


@external
Expand All @@ -54,7 +53,7 @@ def transfer(_to : address, _value : uint256) -> bool:
# so the following subtraction would revert on insufficient balance
self.balanceOf[msg.sender] -= _value
self.balanceOf[_to] += _value
log IERC20.Transfer(msg.sender, _to, _value)
log IERC20.Transfer(sender=msg.sender, receiver=_to, value=_value)
return True


Expand All @@ -73,7 +72,7 @@ def transferFrom(_from : address, _to : address, _value : uint256) -> bool:
# NOTE: vyper does not allow underflows
# so the following subtraction would revert on insufficient allowance
self.allowance[_from][msg.sender] -= _value
log IERC20.Transfer(_from, _to, _value)
log IERC20.Transfer(sender=_from, receiver=_to, value=_value)
return True


Expand All @@ -89,7 +88,7 @@ def approve(_spender : address, _value : uint256) -> bool:
@param _value The amount of tokens to be spent.
"""
self.allowance[msg.sender][_spender] = _value
log IERC20.Approval(msg.sender, _spender, _value)
log IERC20.Approval(owner=msg.sender, spender=_spender, value=_value)
return True


Expand All @@ -106,7 +105,7 @@ def mint(_to: address, _value: uint256):
assert _to != empty(address)
self.totalSupply += _value
self.balanceOf[_to] += _value
log IERC20.Transfer(empty(address), _to, _value)
log IERC20.Transfer(sender=empty(address), receiver=_to, value=_value)


@internal
Expand All @@ -120,7 +119,7 @@ def _burn(_to: address, _value: uint256):
assert _to != empty(address)
self.totalSupply -= _value
self.balanceOf[_to] -= _value
log IERC20.Transfer(_to, empty(address), _value)
log IERC20.Transfer(sender=_to, receiver=empty(address), value=_value)


@external
Expand Down
14 changes: 7 additions & 7 deletions examples/tokens/ERC4626.vy
Original file line number Diff line number Diff line change
Expand Up @@ -56,14 +56,14 @@ def decimals() -> uint8:
def transfer(receiver: address, amount: uint256) -> bool:
self.balanceOf[msg.sender] -= amount
self.balanceOf[receiver] += amount
log IERC20.Transfer(msg.sender, receiver, amount)
log IERC20.Transfer(sender=msg.sender, receiver=receiver, value=amount)
return True


@external
def approve(spender: address, amount: uint256) -> bool:
self.allowance[msg.sender][spender] = amount
log IERC20.Approval(msg.sender, spender, amount)
log IERC20.Approval(owner=msg.sender, spender=spender, value=amount)
return True


Expand All @@ -72,7 +72,7 @@ def transferFrom(sender: address, receiver: address, amount: uint256) -> bool:
self.allowance[sender][msg.sender] -= amount
self.balanceOf[sender] -= amount
self.balanceOf[receiver] += amount
log IERC20.Transfer(sender, receiver, amount)
log IERC20.Transfer(sender=sender, receiver=receiver, value=amount)
return True


Expand Down Expand Up @@ -137,7 +137,7 @@ def deposit(assets: uint256, receiver: address=msg.sender) -> uint256:

self.totalSupply += shares
self.balanceOf[receiver] += shares
log IERC4626.Deposit(msg.sender, receiver, assets, shares)
log IERC4626.Deposit(sender=msg.sender, owner=receiver, assets=assets, shares=shares)
return shares


Expand Down Expand Up @@ -170,7 +170,7 @@ def mint(shares: uint256, receiver: address=msg.sender) -> uint256:

self.totalSupply += shares
self.balanceOf[receiver] += shares
log IERC4626.Deposit(msg.sender, receiver, assets, shares)
log IERC4626.Deposit(sender=msg.sender, owner=receiver, assets=assets, shares=shares)
return assets


Expand Down Expand Up @@ -207,7 +207,7 @@ def withdraw(assets: uint256, receiver: address=msg.sender, owner: address=msg.s
self.balanceOf[owner] -= shares

extcall self.asset.transfer(receiver, assets)
log IERC4626.Withdraw(msg.sender, receiver, owner, assets, shares)
log IERC4626.Withdraw(sender=msg.sender, receiver=receiver, owner=owner, assets=assets, shares=shares)
return shares


Expand All @@ -233,7 +233,7 @@ def redeem(shares: uint256, receiver: address=msg.sender, owner: address=msg.sen
self.balanceOf[owner] -= shares

extcall self.asset.transfer(receiver, assets)
log IERC4626.Withdraw(msg.sender, receiver, owner, assets, shares)
log IERC4626.Withdraw(sender=msg.sender, receiver=receiver, owner=owner, assets=assets, shares=shares)
return assets


Expand Down
10 changes: 5 additions & 5 deletions examples/tokens/ERC721.vy
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ def _transferFrom(_from: address, _to: address, _tokenId: uint256, _sender: addr
# Add NFT
self._addTokenTo(_to, _tokenId)
# Log the transfer
log IERC721.Transfer(_from, _to, _tokenId)
log IERC721.Transfer(sender=_from, receiver=_to, token_id=_tokenId)


### TRANSFER FUNCTIONS ###
Expand Down Expand Up @@ -275,7 +275,7 @@ def approve(_approved: address, _tokenId: uint256):
assert (senderIsOwner or senderIsApprovedForAll)
# Set the approval
self.idToApprovals[_tokenId] = _approved
log IERC721.Approval(owner, _approved, _tokenId)
log IERC721.Approval(owner=owner, approved=_approved, token_id=_tokenId)


@external
Expand All @@ -291,7 +291,7 @@ def setApprovalForAll(_operator: address, _approved: bool):
# Throws if `_operator` is the `msg.sender`
assert _operator != msg.sender
self.ownerToOperators[msg.sender][_operator] = _approved
log IERC721.ApprovalForAll(msg.sender, _operator, _approved)
log IERC721.ApprovalForAll(owner=msg.sender, operator=_operator, approved=_approved)


### MINT & BURN FUNCTIONS ###
Expand All @@ -313,7 +313,7 @@ def mint(_to: address, _tokenId: uint256) -> bool:
assert _to != empty(address)
# Add NFT. Throws if `_tokenId` is owned by someone
self._addTokenTo(_to, _tokenId)
log IERC721.Transfer(empty(address), _to, _tokenId)
log IERC721.Transfer(sender=empty(address), receiver=_to, token_id=_tokenId)
return True


Expand All @@ -333,7 +333,7 @@ def burn(_tokenId: uint256):
assert owner != empty(address)
self._clearApproval(owner, _tokenId)
self._removeTokenFrom(owner, _tokenId)
log IERC721.Transfer(owner, empty(address), _tokenId)
log IERC721.Transfer(sender=owner, receiver=empty(address), token_id=_tokenId)


@view
Expand Down
43 changes: 43 additions & 0 deletions tests/functional/syntax/test_event_kwarg_hint.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import warnings

from vyper.compiler import compile_code


def test_event_kwarg_hint():
code = """
from ethereum.ercs import IERC20

def foo():
log IERC20.Transfer(msg.sender, msg.sender, 123)
"""

with warnings.catch_warnings(record=True) as w:
assert compile_code(code) is not None

expected = "Instantiating events with positional arguments is deprecated "
expected += "as of v0.4.1 and will be disallowed in a future release. "
expected += "Use kwargs instead e.g.:\n"
expected += "```\nlog IERC20.Transfer(sender=msg.sender, receiver=msg.sender, value=123)\n```"

assert len(w) == 1, [s.message for s in w]
assert str(w[0].message).startswith(expected)


def test_event_hint_single_char_argument():
code = """
from ethereum.ercs import IERC20

def foo():
log IERC20.Transfer(msg.sender, msg.sender, 1)
"""

with warnings.catch_warnings(record=True) as w:
assert compile_code(code) is not None

expected = "Instantiating events with positional arguments is deprecated "
expected += "as of v0.4.1 and will be disallowed in a future release. "
expected += "Use kwargs instead e.g.:\n"
expected += "```\nlog IERC20.Transfer(sender=msg.sender, receiver=msg.sender, value=1)\n```"

assert len(w) == 1, [s.message for s in w]
assert str(w[0].message).startswith(expected)
Loading
Loading