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

【LocalNouns】ミント条件の整備 #157

Merged
merged 17 commits into from
Oct 27, 2023

Conversation

EibaKatsu
Copy link
Collaborator

主にMinterにSalePhaseによるチェックと、TokenGateによるチェックを追加しました。
addressファイルをコミットしてしまったため数が多くなっちゃいました🙏

この後、購入金額チェックや都道府県ごとにミント上限数を設定していきます。

@isamu isamu requested review from snakajima and isamu October 25, 2023 23:12
}

function mintSelectedPrefectureBatch(
function mintSelectedPrefecture(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

memo
minterが指定する県をnのmintする。
LocalNounsProvider.solがよばれる。
mint(uint256 prefectureId, uint256 _assetId)

1,2桁目:都道府県番号、3桁目以降:バージョン番号??

Copy link
Contributor

@isamu isamu Oct 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@EibaKatsu
150, 73など、2桁が都道府県番号じゃない場合にはどうなりますか?
どこかでvalidationいれたほうがよさそうですね。

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

「バージョン番号??」は使ってないように見えますが必要ですか?
必要でないのであれば0 ~ 47指定にして0ならランダムのほうが良いですね。

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LocalNounsMinter.solのmintSelectedPrefectureから呼び出しいるのだと思いますが、
ここはpriceがあるので一般ユーザが使うと理解しています。
であれば
require(msg.sender == minter, 'Sender is not the minter');
があるので、呼べないとなりませんか?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

「バージョン番号??」は使ってないように見えますが必要ですか? 必要でないのであれば0 ~ 47指定にして0ならランダムのほうが良いですね。

初回セールではバージョンを使用しません。
今後HEADパーツが追加された場合に、追加したパーツだけのセールをしたいのでバージョンを設けています。

filenameの先頭がバージョン+都道府県番号で以下のイメージになります。

バージョンなし(初回)
image

バージョンあり(パーツ追加後)
image

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1,2桁目:都道府県番号、3桁目以降:バージョン番号??

表現がおかしいですね、、
下2桁:都道府県番号、下3桁より上位:バージョン番号です。
コメント修正しておきます。

Copy link
Collaborator Author

@EibaKatsu EibaKatsu Oct 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@EibaKatsu 150, 73など、2桁が都道府県番号じゃない場合にはどうなりますか? どこかでvalidationいれたほうがよさそうですね。

追加しました。55e49ad

追記:
存在しないバージョン+都道府県番号だとミント時にエラーになるので、厳密な存在チェックは割愛します。

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LocalNounsMinter.solのmintSelectedPrefectureから呼び出しいるのだと思いますが、 ここはpriceがあるので一般ユーザが使うと理解しています。 であれば require(msg.sender == minter, 'Sender is not the minter'); があるので、呼べないとなりませんか?

mintPrice, mintLimit のミント条件は、Minterコントラクト側で制御します。
この変数を使用しないことをコメントしておきました。

1f91eed
2331227

@EibaKatsu EibaKatsu merged commit 0000b9c into Cryptocoders-wtf:main Oct 27, 2023
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants