Skip to content

pull request with suggestions#1

Open
hananbeer wants to merge 1 commit into0xBeans:masterfrom
hananbeer:pr
Open

pull request with suggestions#1
hananbeer wants to merge 1 commit into0xBeans:masterfrom
hananbeer:pr

Conversation

@hananbeer
Copy link
Copy Markdown

mainly some small semantic changes for clarity.

one thing I did change was adding a block limit of 100 blocks after PoS.
for PoW you can mint until PoS.

I also addes very minor tests. I will need to consult you regarding the renderer, it looks good in the code but I'm unfamiliar with these. but anyways we can always setRenderer so I'm not worried about that.

good work, I believe we're good to go!

cache/
out/
artifacts/
lib/
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

typically we want to keep lib so ppl can just run forge install when playing with the repo

@@ -1,5 +1,5 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.13;
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

can you add this back

}

constructor() ERC721("Conclusion", "CONCLUSION") {}
constructor() ERC721("The Last Work", "CONCLUSION") {}
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

lets do 'sunset' and 'sunrise' for POS

if (!mergeHasOccured()) {
lastWorkBlock = block.number;
}
function _assetPoW() internal {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

asset or assess?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

assert

}

constructor() ERC721("Genesis", "GENESIS") {}
constructor() ERC721("New Genesis", "GENESIS") {}
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

sunrise


function checkForMergeAndUpdate() public {
if (genesisMergeBlock == 0 && mergeHasOccured()) {
function assertPoS() public {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

i was thinking we could make this a function anyone could call just so we can keep updating the blocks (so we can guarantee not missing the POS block)

info.blockNum,
info.blockdifficulty
info.blockNumber,
genesisMergeBlock
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

this is suppose to be the block difficulty the minter minted at - for metadata purposes

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

PoS doesn't have block.difficulty (it's prevrandao then) so I re-purposed it to show when genesisMergeBlock (since it isn't known while PoW, so it's a nice compromise)

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