Implement Memes token contract#59
Conversation
kmlee307
force-pushed
the
meme-token
branch
2 times, most recently
from
68a3fe9 to
08d5281
Compare
happyhappy-jun
left a comment
happyhappy-jun
left a comment
There was a problem hiding this comment.
참고 링크를 찾아서 첨부합니다!
- 전체적으로 tx 가 실행되기 전에 체크해야하는 것들을 추가해야할꺼 같습니다. (ex. 옮기기전에 충분한 돈이 있는지? 등)
- 전체 발행량을 쿼리 할 수 있는 함수를 만들어주세요 (만약에 near_token_standard 에 이미 구현되었다면, 어떤 함수를 쓰면 되는지 댓글로 달아주세요!)
| near_contract_standards::impl_fungible_token_storage!(WhaleContract, token); | ||
|
|
||
| ///TODO: replace PDAO's whale icon | ||
| const PDAO_WHALE_ICON: &str = "PDAO WHALE ICON ADRESSS"; |
There was a problem hiding this comment.
정확하게 니어 토큰이 이미지 데이터를 어떻게 받는지는 모르겠지만, 예시 코드에서는 svg 포맷이였거든요. 해당 포맷만 억셉하는지, 아니면 다른 데이터 타입 (base64로 인코딩 된 이미지, 이미지 링크) 도 가능한지, 리서치해서 주석으로 남겨주실수 있나요?
| pub fn new() -> Self { | ||
| Self { | ||
| token: FungibleToken::new(b"w".to_vec()), | ||
| decimals: 8, |
There was a problem hiding this comment.
decimal 은 임의 설정된 값인가요?
There was a problem hiding this comment.
네, 일단의 소스코드와 동일하게 설정했습니다.
정해진 값이 있을까요?
There was a problem hiding this comment.
정해진 값은 없지만, 니어 프로토콜에서 사용하는 1 Near = 10^24 yoctoNear 라는 단위가 있으니, 여기에 24 decimal 로 맞추는게 좋을 듯 합니다
https://github.com/near-examples/FT/blob/master/ft/src/lib.rs
| pub fn burn(&mut self, account_id: AccountId, amount: U128) { | ||
| // validate with lightclient | ||
| self.validate_with_lightclient(); | ||
| self.token.internal_withdraw(&account_id, amount.into()); |
There was a problem hiding this comment.
토큰의 burn 은 해당 토큰을 완전히 블록체인 상에서 지워버리는 겁니다.
FT의 경우 내가 가진 WHALE 과 네가 가진 WHALE 은 fungible 하니 withdraw 한 뒤에 전체 발행량에서 태운 만큼을 줄이는 로직이 필요해보입니다.
https://github.com/near/rainbow-bridge-fun/blob/66ff98d2615d10cf57f0dec31d19480cafd570bc/mintable-fungible-token/src/lib.rs 여기 참고해서 해당 기능 추가해주세요
There was a problem hiding this comment.
near_contract_standards의 internal_withdraw를 사용하면 전체 발행량에서도 삭제되는 것으로 보입니다. burn test에서 같이 확인할 수 있도록 수정하겠습니다.
| } | ||
|
|
||
| #[test] | ||
| fn double_minting() { |
There was a problem hiding this comment.
- 테스트는
test_prefix 로 시작하게 해주세요 - 이 테스트는 무엇을 테스트 하고 있나요?
There was a problem hiding this comment.
동일한 계정으로 여러번 minting하는 경우를 테스트 합니다
There was a problem hiding this comment.
네네 근데 test_minting 함수로 충분히 테스트 되는 영역 아닐가요? 1이 독립적으로 정상 작동하는 것을 확인했는데, 1+1 이 잘 되는지 확인하는게 필요할까? 하는 생각입니다. 만약에 이게 안되는 로직이 있다면 그 때 두번 민팅하는게 필요하지 않을까 합니다.
There was a problem hiding this comment.
이미 한번 민팅되어 등록된 계정을 다시 등록하게 되면 초기화되는데 등록없이 추가 민팅하는 것을 보고 싶어 추가했습니다. 굳이 필요없으신 것 같다면 삭제하겠습니다.
style: modify and add comments refactor: remove unused import feat: add minting function and validate with lightclient before mint and transfer style: delete unused import and fix comments refactor: delete unnecessary function refactor: change contract struct test: fix test for minting and transfer and add test for burn and double minting style: edit import style: fix for yarn format
| } | ||
|
|
||
| pub fn transfer_whale(&mut self, receiver_id: AccountId, amount: U128, memo: Option<String>) { | ||
| //before transfer validate with lightclient |
There was a problem hiding this comment.
주석 // 다음에는 whitespace 한칸을 두는걸 규칙으로 합시다.
|
|
||
| #[test] | ||
| #[should_panic(expected = "The contract is not initialized")] | ||
| fn test_default() { |
There was a problem hiding this comment.
테스트 함수들에 test_를 붙이지 않는걸 컨벤션으로 합시다.
동사로 뭘 하는지 나타내는게 더 간결할 것 같네요. (e.g., mint_1(), mint_and_butn_1() 등
| memo: Option<String>, | ||
| msg: String, | ||
| ) -> PromiseOrValue<U128> { | ||
| self.validate_with_lightclient(); |
There was a problem hiding this comment.
transfer는 라클을 거치는게 아닙니다. 그냥 자유롭게 할 수 있어야돼요.
3 participants
Type of this PR
Describe your changes
Issue ticket number and other helpful resource
#54
https://github.com/near-examples/FT/tree/master/ft/src
https://github.com/tonic-foundation/tonic-core/blob/31f8c72493aae28bfb37b90b5319c3ffb8879832/test-token/src/lib.rs
Checklist before requesting a review
Checklist after creating a pull request