Skip to content

Commit

Permalink
refactor: apply recommandations
Browse files Browse the repository at this point in the history
  • Loading branch information
skimaharvey committed Nov 28, 2024
1 parent fb6c644 commit 0b8eaa0
Show file tree
Hide file tree
Showing 6 changed files with 62 additions and 45 deletions.
16 changes: 8 additions & 8 deletions packages/lsp7-contracts/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,6 @@ export const INTERFACE_ID_LSP7_PREVIOUS = {
};

export const LSP7_TYPE_IDS = {
// keccak256('LSP7Tokens_DelegatorNotification')
LSP7Tokens_DelegatorNotification:
'0x997bd66a7e7823b09383ec7ce65fc306af29b8f82a45627f8efc0408475de016',

// keccak256('LSP7Tokens_DelegateeNotification')
LSP7Tokens_DelegateeNotification:
'0x03fae98a28026f93c23e2c9438c2ef0faa101585127a89919d18f067d907b319',

// keccak256('LSP7Tokens_SenderNotification')
LSP7Tokens_SenderNotification:
'0x429ac7a06903dbc9c13dfcb3c9d11df8194581fa047c96d7a4171fc7402958ea',
Expand All @@ -25,4 +17,12 @@ export const LSP7_TYPE_IDS = {
// keccak256('LSP7Tokens_OperatorNotification')
LSP7Tokens_OperatorNotification:
'0x386072cc5a58e61263b434c722725f21031cd06e7c552cfaa06db5de8a320dbc',

// keccak256('LSP7Tokens_VotesDelegatorNotification')
LSP7Tokens_VotesDelegatorNotification:
'0x6117a486162c4ba8e38d646ef52b1e0e1be6bef05a980c041e232eba8c95e16f',

// keccak256('LSP7Tokens_VotesDelegateeNotification')
LSP7Tokens_VotesDelegateeNotification:
'0x72cad372b29cde295ff0839b7b194597766b88f5fad4f7d6aef013e0c55dc492',
};
24 changes: 12 additions & 12 deletions packages/lsp7-contracts/contracts/LSP7DigitalAsset.sol
Original file line number Diff line number Diff line change
Expand Up @@ -762,46 +762,46 @@ abstract contract LSP7DigitalAsset is
* (or `to`) is the zero address. All customizations to transfers, mints, and burns should be done by overriding
* this function.
*
* Emits a {Transfer} event.
* @custom:events {Transfer} event.
*/
function _update(
address from,
address to,
uint256 value,
uint256 amount,
bool force,
bytes memory data
) internal virtual {
if (from == address(0)) {
// Overflow check required: The rest of the code assumes that totalSupply never overflows
_existingTokens += value;
_existingTokens += amount;
} else {
uint256 fromBalance = _tokenOwnerBalances[from];
if (fromBalance < value) {
revert LSP7AmountExceedsBalance(fromBalance, from, value);
if (fromBalance < amount) {
revert LSP7AmountExceedsBalance(fromBalance, from, amount);
}
unchecked {
// Overflow not possible: value <= fromBalance <= totalSupply.
_tokenOwnerBalances[from] = fromBalance - value;
// Overflow not possible: amount <= fromBalance <= totalSupply.
_tokenOwnerBalances[from] = fromBalance - amount;
}
}

if (to == address(0)) {
unchecked {
// Overflow not possible: value <= totalSupply or value <= fromBalance <= totalSupply.
_existingTokens -= value;
// Overflow not possible: amount <= totalSupply or amount <= fromBalance <= totalSupply.
_existingTokens -= amount;
}
} else {
unchecked {
// Overflow not possible: balance + value is at most totalSupply, which we know fits into a uint256.
_tokenOwnerBalances[to] += value;
// Overflow not possible: balance + amount is at most totalSupply, which we know fits into a uint256.
_tokenOwnerBalances[to] += amount;
}
}

emit Transfer({
operator: msg.sender,
from: from,
to: to,
amount: value,
amount: amount,
force: force,
data: data
});
Expand Down
22 changes: 11 additions & 11 deletions packages/lsp7-contracts/contracts/LSP7DigitalAssetInitAbstract.sol
Original file line number Diff line number Diff line change
Expand Up @@ -781,41 +781,41 @@ abstract contract LSP7DigitalAssetInitAbstract is
function _update(
address from,
address to,
uint256 value,
uint256 amount,
bool force,
bytes memory data
) internal virtual {
if (from == address(0)) {
// Overflow check required: The rest of the code assumes that totalSupply never overflows
_existingTokens += value;
_existingTokens += amount;
} else {
uint256 fromBalance = _tokenOwnerBalances[from];
if (fromBalance < value) {
revert LSP7AmountExceedsBalance(fromBalance, from, value);
if (fromBalance < amount) {
revert LSP7AmountExceedsBalance(fromBalance, from, amount);
}
unchecked {
// Overflow not possible: value <= fromBalance <= totalSupply.
_tokenOwnerBalances[from] = fromBalance - value;
// Overflow not possible: amount <= fromBalance <= totalSupply.
_tokenOwnerBalances[from] = fromBalance - amount;
}
}

if (to == address(0)) {
unchecked {
// Overflow not possible: value <= totalSupply or value <= fromBalance <= totalSupply.
_existingTokens -= value;
// Overflow not possible: amount <= totalSupply or amount <= fromBalance <= totalSupply.
_existingTokens -= amount;
}
} else {
unchecked {
// Overflow not possible: balance + value is at most totalSupply, which we know fits into a uint256.
_tokenOwnerBalances[to] += value;
// Overflow not possible: balance + amount is at most totalSupply, which we know fits into a uint256.
_tokenOwnerBalances[to] += amount;
}
}

emit Transfer({
operator: msg.sender,
from: from,
to: to,
amount: value,
amount: amount,
force: force,
data: data
});
Expand Down
2 changes: 1 addition & 1 deletion packages/lsp7-contracts/contracts/Mocks/MyVotingToken.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {LSP7Votes, LSP7DigitalAsset} from "../extensions/LSP7Votes.sol";
import {EIP712} from "@openzeppelin/contracts/utils/cryptography/EIP712.sol";

/**
* @dev Mock of an LSP7Votes token
* @dev Mock of a LSP7Votes token
*/
contract MyVotingToken is LSP7Votes {
constructor()
Expand Down
37 changes: 25 additions & 12 deletions packages/lsp7-contracts/contracts/extensions/LSP7Votes.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,6 @@ pragma solidity ^0.8.0;

import {LSP7DigitalAsset} from "../LSP7DigitalAsset.sol";
import {LSP1Utils} from "@lukso/lsp1-contracts/contracts/LSP1Utils.sol";
import {
_TYPEID_LSP7_DELEGATOR,
_TYPEID_LSP7_DELEGATEE
} from "../LSP7Constants.sol";
import {IERC5805} from "@openzeppelin/contracts/interfaces/IERC5805.sol";
import {Math} from "@openzeppelin/contracts/utils/math/Math.sol";
import {SafeCast} from "@openzeppelin/contracts/utils/math/SafeCast.sol";
Expand Down Expand Up @@ -38,6 +34,14 @@ abstract contract LSP7Votes is LSP7DigitalAsset, EIP712, IERC5805 {
bytes32 private constant _DELEGATION_TYPEHASH =
keccak256("Delegation(address delegatee,uint256 nonce,uint256 expiry)");

// keccak256('LSP7Tokens_VotesDelegatorNotification')
bytes32 constant _TYPEID_LSP7_VOTESDELEGATOR =
0x6117a486162c4ba8e38d646ef52b1e0e1be6bef05a980c041e232eba8c95e16f;

// keccak256('LSP7Tokens_VotesDelegateeNotification')
bytes32 constant _TYPEID_LSP7_VOTESDELEGATEE =
0x72cad372b29cde295ff0839b7b194597766b88f5fad4f7d6aef013e0c55dc492;

mapping(address => address) private _delegates;
mapping(address => Checkpoint[]) private _checkpoints;
Checkpoint[] private _totalSupplyCheckpoints;
Expand Down Expand Up @@ -107,8 +111,7 @@ abstract contract LSP7Votes is LSP7DigitalAsset, EIP712, IERC5805 {
/**
* @dev Retrieve the number of votes for `account` at the end of `timepoint`.
*
* Requirements:
*
* @custom:requirement
* - `timepoint` must be in the past
*/
function getPastVotes(
Expand All @@ -123,8 +126,7 @@ abstract contract LSP7Votes is LSP7DigitalAsset, EIP712, IERC5805 {
* @dev Retrieve the `totalSupply` at the end of `timepoint`. Note, this value is the sum of all balances.
* It is NOT the sum of all the delegated votes!
*
* Requirements:
*
* @custom:requirement
* - `timepoint` must be in the past
*/
function getPastTotalSupply(
Expand Down Expand Up @@ -243,7 +245,9 @@ abstract contract LSP7Votes is LSP7DigitalAsset, EIP712, IERC5805 {
/**
* @dev Move voting power when tokens are transferred.
*
* Emits a {IVotes-DelegateVotesChanged} event.
* @custom:event
* - {IVotes-DelegateVotesChanged} when voting power is removed from source address
* - {IVotes-DelegateVotesChanged} when voting power is added to destination address
*/
function _update(
address from,
Expand All @@ -267,7 +271,9 @@ abstract contract LSP7Votes is LSP7DigitalAsset, EIP712, IERC5805 {
/**
* @dev Change delegation for `delegator` to `delegatee`.
*
* Emits events {IVotes-DelegateChanged} and {IVotes-DelegateVotesChanged}.
* @custom:event
* - {IVotes-DelegateChanged}
* - {IVotes-DelegateVotesChanged}
*/
function _delegate(address delegator, address delegatee) internal virtual {
address currentDelegate = delegates(delegator);
Expand All @@ -287,7 +293,7 @@ abstract contract LSP7Votes is LSP7DigitalAsset, EIP712, IERC5805 {
);
LSP1Utils.notifyUniversalReceiver(
delegator,
_TYPEID_LSP7_DELEGATOR,
_TYPEID_LSP7_VOTESDELEGATOR,
delegatorNotificationData
);
}
Expand All @@ -302,12 +308,19 @@ abstract contract LSP7Votes is LSP7DigitalAsset, EIP712, IERC5805 {

LSP1Utils.notifyUniversalReceiver(
delegatee,
_TYPEID_LSP7_DELEGATEE,
_TYPEID_LSP7_VOTESDELEGATEE,
delegateeNotificationData
);
}
}

/**
* @dev Moves voting power from one address to another.
*
* @custom:event
* - {IVotes-DelegateVotesChanged} when voting power is removed from source address
* - {IVotes-DelegateVotesChanged} when voting power is added to destination address
*/
function _moveVotingPower(
address src,
address dst,
Expand Down
6 changes: 5 additions & 1 deletion packages/lsp7-contracts/tests/LSP7Votes.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,11 @@ describe('Comprehensive Governor and Token Tests', () => {

await expect(token.connect(voter1).delegate(await mockUniversalReceiver.getAddress()))
.to.emit(mockUniversalReceiver, 'UniversalReceiverCalled')
.withArgs(token.target, LSP7_TYPE_IDS.LSP7Tokens_DelegateeNotification, expectedData);
.withArgs(
token.target,
LSP7_TYPE_IDS.LSP7Tokens_VotesDelegateeNotification,
expectedData,
);
});

it('should not notify delegatee when delegator has zero balance', async () => {
Expand Down

0 comments on commit 0b8eaa0

Please sign in to comment.