[Siyoung] Vote Contract 작성 #4
Conversation
1. deployer -> owner 용어 변경 2. require 소유권자 검증이 반복되어 함수변경자로 적용 (onlyOwner) 3. startTime, endTime 적용 - 현재 시간이 start, end time 사이에 있을 때만 vote 함수 실행 가능 => voteOpen(함수변경자) - 그 외 함수는 투표 시작시간 전에 배포해놓고 투표자를 세팅할 수도 있으므로 검증 제외 4. 임시 BadgemealNFT 컨트랙트를 통해 voters 매핑에 추가 전 NFT 홀더인지 검증 5. Vote Contract를 한번 배포해서 계속 쓰는 것이 아니라 투표 때마다 새로운 Vote Contract를 배포해서 사용 (투표 히스토리 목적) 6. 투표 종료 시간 체크는 백엔드에서 스케쥴러로 체크. 백엔드에서 winnerProposal 확인 후 BadgemealNFT 컨트랙트에 addWinnerProposals 함수로 메뉴 추가
| - 그 외 함수는 투표 시작시간 전에 배포해놓고 투표자를 세팅할 수도 있으므로 시간 검증 제외 | ||
| - 임시 BadgemealNFT 컨트랙트를 통해 voters 매핑에 추가 전 NFT 홀더인지 검증 | ||
| - Vote Contract를 한번 배포해서 계속 쓰는 것이 아니라 투표 때마다 새로운 Vote Contract를 배포해서 사용 (투표 히스토리 관리, 투명성) |
There was a problem hiding this comment.
투표 히스토리를 투명하게 관리하는 목적은 좋습니다. 다만 매번 새로운 컨트랙트를 배포하기 보다는 어짜피 컨트랙트 내에서 발생되는 트랜잭션으로 투명성을 확보할 수 있기 때문에 , 굳이 매번 새로 배포할 필요는 없을 것 같습니다!
There was a problem hiding this comment.
투표마다 새로운 컨트랙트 배포에서 한번 배포 후 재사용 방식으로 변경하였습니다.
| struct Proposal { | ||
| string name; // 메뉴 이름 | ||
| string imageUrl; // 메뉴 이미지 url |
There was a problem hiding this comment.
Proposal 에 imageUrl은 필요 없을 것 같습니다. 회의를 진행한 내용에 따르면 Proposal의 name을 가지고 기존 이미지에 텍스트만 교체하여 NFT를 발행하기로 했기 때문입니다. 따라서 채택된 메뉴가 DB에 저장될때 해당 메뉴에 부합하는 이미지를 자체적으로 매핑해서 관리하면 될 것 같습니다.
| msg.sender == owner, | ||
| "msg sender is not a owner." | ||
| ); |
There was a problem hiding this comment.
Owner를 체크하는 함수는 Ownable 컨트랙트를 상속받아서 사용하면 편하게 사용할 수 있습니다!
https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/access/Ownable.sol
| function addNFTHolders(address[] memory _addresses) public onlyOwner { | ||
| for (uint i = 0; i < _addresses.length; i++) { | ||
| addNFTHolder(_addresses[i]); |
There was a problem hiding this comment.
추측하기로는 parameter인 _addresses가 전체 뱃지밀 NFT 홀더의 address 리스트인 것 같은데요. 만약 홀더가 100명, 1000명 이상 되면 상당한 가스비를 소모할 것으로 예상됩니다. 최대한 실행이 많이 반복되는 코드는 줄이는 것이 좋을 것 같아요!
| require( | ||
| BadgemealNFT(nftAddress).isHolder(_address), | ||
| concat(toString(_address), " is not NFT Holder.") |
There was a problem hiding this comment.
어떤 address가 인자로 들어왔는지 에러메세지에 함께 보여주는 부분이 좋은 것 같아요! 👍
| // address -> string 변환 함수 | ||
| function toString(address account) internal pure returns(string memory) { | ||
| return toString(abi.encodePacked(account)); |
| // string concat 함수 | ||
| function concat(string memory a, string memory b) internal pure returns (string memory) { | ||
| return string(abi.encodePacked(a, b)); |
| uint startTime; // 투표 시작 시간 | ||
|
|
||
| uint endTime; // 투표 마감 시간 |
There was a problem hiding this comment.
startTime, endTime 을 세팅하는 아이디어 좋습니다 👍
| owner = msg.sender; | ||
| startTime = _startTime; | ||
| endTime = _endTime; |
There was a problem hiding this comment.
생성자에서 startTime, endTime 을 세팅하기보다 백엔드 스케줄러를 통해서 시작 전날 세팅하는 함수를 만드는 것도 좋을 것 같습니다!
There was a problem hiding this comment.
투표 컨트랙트 배포해서 하나로 계속 쓰는 방식이면 수빈님 말대로 하는게 맞는 것 같습니다.
| require( | ||
| now >= startTime && now < endTime, | ||
| "voting currently not open" |
| - desc: 투표자로 추가하려한 대상 address를 error message에 출력함으로써 어떤 address가 문제인지 메시지를 통해 확인 가능 | ||
| ```sol | ||
| function addVoter(address _address, address nftAddress) private { |
There was a problem hiding this comment.
투표할 사람들을 미리 addVoter에 추가하고 투표한 사람들마다 처리하도록 같은데 맞나요?
매번 마스터 NFT가 민팅될때마다 addVoter를 호출해야해서 비효율적일 수 있을 것 같아요.
혹은 투표를 시간한 시점 이후에 마스터 NFT를 받은 사람은 투표자 명단에서 제외 될 수도 있고요.
투표할 때마다 NFT 소유자인지 아닌지는 vote function에서 확인하고 투표를 한 사람만 addVoter로 추가하면 좋을 것같아요
There was a problem hiding this comment.
미리 voters에 추가하는 것은 아닙니다. 매번 투표자가 투표할 때마다 트랜잭션을 발생시키는 것보다 클라이언트단에서 투표 결과를 투표 종료전까지 DB로 저장한 다음 투표가 종료되면 한번의 트랜잭션으로 투표한 사람들의 address를 리스트로 넘겨 voters 매핑에 추가하고 투표결과를 vote함수로 반영하는 것으로 생각했습니다.
현기님 말씀대로 투표자가 투표할 때마다 검증하는 방식으로 한다면 마스터 NFT를 받으면서 생기는 투표자 유효여부에 따른 이슈를 줄일 수 있을 것 같네요.
|
|
||
| struct Voter { | ||
| bool exist; // 투표자 존재 여부 (true,false) |
There was a problem hiding this comment.
voters mapping에서 투표자 address가 매핑에 등록된 것인지 확인하는 과정에서 voters[address]가 null값이어도 리턴으로 Voter 구조체가 초기화된 값으로 나오길래 존재여부 변수(exist)를 추가하여 조건문에서 사용했습니다.
|
고생하셨습니다👍 |
1. 투표 때마다 vote 컨트랙트 새로 배포 -> vote 컨트랙트 한번 배포 후 재사용 2. addVoters, addVoter 함수 제거 3. Ownable 컨트랙트 상속으로 컨트랙트 최적화 4. imageUrl 제거 5. vote함수에 조건 추가 (msg.sender가 NFT홀더인 경우)
# Conflicts: # voteContract.sol
No description provided.