-
Notifications
You must be signed in to change notification settings - Fork 13
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
add SP3QSAJQ4EA8WXEDSRRKMZZ29NH91VZ6C5X88FGZQ/crashpunks-v1 #1
add SP3QSAJQ4EA8WXEDSRRKMZZ29NH91VZ6C5X88FGZQ/crashpunks-v1 #1
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Experimenting with commenting deployed contracts in github
) | ||
) | ||
(define-read-only (get-sale-data (nftIndex uint)) | ||
(match (map-get? nft-sale-data {nft-index: nftIndex}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use unwrap!
for better readability
) | ||
) | ||
|
||
;; read only methods |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
most read-only functions do not need a result of type response
, better to use plain value if not required by traits.
Specifying read-only in traits might change with Stacks 2.1: stacks-network/stacks-core#1981
(tokenSymbol the-token-symbol))) | ||
) | ||
) | ||
(define-private (get-all-data (nftIndex uint)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could be read-only
(+ split (unwrap! (pay-royalty payer myMintPrice (unwrap! (element-at mintAddresses u1) payment-address-error) (unwrap! (element-at mintShares u1) payment-share-error)) payment-share-error)) | ||
(+ split (unwrap! (pay-royalty payer myMintPrice (unwrap! (element-at mintAddresses u2) payment-address-error) (unwrap! (element-at mintShares u2) payment-share-error)) payment-share-error)) | ||
(+ split (unwrap! (pay-royalty payer myMintPrice (unwrap! (element-at mintAddresses u3) payment-address-error) (unwrap! (element-at mintShares u3) payment-share-error)) payment-share-error)) | ||
;; (print {evt: "paymint-split", nftIndex: nftIndex, payer: payer, mintPrice: myMintPrice, txSender: tx-sender}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove unused code
) | ||
;; ignore royalty payment if its to the buyer / tx-sender. | ||
(if (not (is-eq tx-sender payee)) | ||
(unwrap! (stx-transfer? split payer payee) transfer-error) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Information about the transfer error is lost. Is that intentional? If there is only one execution path that returns error code between 1 and 4, use (try! (stx-transfer? ...))
;; mint twenty tokens | ||
(define-public (collection-mint-token-twenty (signature (buff 65)) (message (buff 32)) (hashes (list 20 (buff 32))) (meta-urls (list 20 (string-ascii 256))) (maxEditions uint) (editionCost uint) (clientMintPrice uint) (buyNowPrice uint)) | ||
(begin | ||
(unwrap! (collection-mint-token signature message (unwrap! (element-at hashes u0) not-allowed) (unwrap! (element-at meta-urls u0) not-allowed) maxEditions editionCost clientMintPrice buyNowPrice) not-allowed) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
message
is the same for the 20 asset hashes. Bad actors could front run and mint with the same message
and signature
.
) | ||
|
||
(define-public (transfer-memo (id uint) (sender principal) (recipient principal) (memo (buff 34))) | ||
(let ((result (transfer id sender recipient))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alternative
(begin
(try! (transfer id sender recipient))
(print memo)
(ok true))
(map-set nft-sale-data { nft-index: nftIndex } { sale-cycle-index: (+ saleCycleIndex u1), sale-type: u0, increment-stx: u0, reserve-stx: u0, amount-stx: u0, bidding-end-time: u0}) | ||
;; finally transfer ownership to the buyer (note: via the buyers transaction!) | ||
(print {evt: "buy-now", nftIndex: nftIndex, owner: owner, recipient: recipient, amount: amount}) | ||
(nft-transfer? crashpunks nftIndex owner recipient) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
buy-now
is not guarded once the sale data is set
(define-public (buy-now (nftIndex uint) (owner principal) (recipient principal)) | ||
(let | ||
( | ||
(saleType (unwrap! (get sale-type (map-get? nft-sale-data {nft-index: nftIndex})) amount-not-set)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extract (unwrap! (map-get? nft-sale-data {nft-index: nftIndex}) err-not-found)
into a variable for better readability
) | ||
) | ||
|
||
(define-public (collection-mint-token (signature (buff 65)) (message-hash (buff 32)) (asset-hash (buff 32)) (metaDataUrl (string-ascii 256)) (maxEditions uint) (editionCost uint) (clientMintPrice uint) (buyNowPrice uint)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
metaDataUrl
can be set by users freely. That value is returned by get-token-uri
(err code))))) | ||
|
||
;; see operable-trait | ||
(define-public (set-approved (nftIndex uint) (operator principal) (approved bool)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function should allow to set-approved for nfts owned at any time, not just right now.
) | ||
) | ||
|
||
(define-read-only (is-approved (nftIndex uint) (address principal)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Naming can be confusing because this function always approves any address
if the caller is the tx-sender
or contract-caller
nft index starts at 0 in this contract - should this be changed to 1 ? |
No description provided.