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

BIP94 Testnet 4 #1601

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

BIP94 Testnet 4 #1601

wants to merge 1 commit into from

Conversation

fjahr
Copy link
Contributor

@fjahr fjahr commented May 27, 2024

A written specification for Testnet 4 has been requested in the PR discussion and a BIP seems to be the right place for this.

Given the active discussion in the PR I think it's best if this is kept open until the PR is merged. I will keep it up-to-date and track the current status of the PR.

- Removal of the 20-min difficulty exception was discussed but dismissed since several reviewers insisted that it was a useful feature.
- Increase of minimum difficulty was discussed but dismissed as it would categorically prevent participation in the network using a CPU miner.
- Increase of the delay in the 20 min difficulty exception was suggested but did not receive significant support.
- Re-enabling `acceptnonstdtxn` by default on the network was dismissed as it had led to confusion among layer-2s that had used testnet for transaction propagation tests.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd consider Bitcoin Core's default policy to be out of scope for this BIP. It's completely unrelated to launching a new testnet, and can be discussed and changed at any later time.

Copy link
Member

@Sjors Sjors May 28, 2024

Choose a reason for hiding this comment

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

It's fine to mention this in the rationale section. The actual specification can leave it up to implementers to decide what to do (MAY, rather than SHOULD or MUST), but explaining pros and cons is useful.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, in Testnet 3 it was changed long after it's launch. But since it was part of the conversation and considered a potential component in the bug/feature discussion of the 20-min exception and whether that should be kept or removed, I think it's important to mention in rationale.

bip-testnet4.mediawiki Outdated Show resolved Hide resolved
bip-testnet4.mediawiki Outdated Show resolved Hide resolved
bip-testnet4.mediawiki Outdated Show resolved Hide resolved
@Sjors
Copy link
Member

Sjors commented May 28, 2024

Thanks for writing a BIP, added to review list.

@fjahr
Copy link
Contributor Author

fjahr commented May 28, 2024

Addressed feedback by @vostrnad , thanks for the review!

Copy link
Contributor

@murchandamus murchandamus left a comment

Choose a reason for hiding this comment

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

Looks pretty good so far, but waiting for this to get more review and be presented on the mailing list.

bip-testnet4.mediawiki Outdated Show resolved Hide resolved
bip-testnet4.mediawiki Outdated Show resolved Hide resolved
@murchandamus murchandamus added New BIP PR Author action required Needs updates, has unaddressed review comments, or is otherwise waiting for PR author labels May 28, 2024
@fjahr fjahr force-pushed the testnet4 branch 3 times, most recently from 18e1080 to 9ecaea0 Compare May 28, 2024 23:44
bip-testnet4.mediawiki Outdated Show resolved Hide resolved

=== Message Start ===

The message start is defined as <code>0x1c163f28</code>. These four bytes were randomly generated and have no special meaning.
Copy link
Member

@maflcko maflcko May 30, 2024

Choose a reason for hiding this comment

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

nit: Instead of claiming that they are randomly generated, it may be better to generate them deterministically, so that the same approach can be used for testnet5?

E.g. echo -n testnet4 | sha256sum | head -c 8, or similar, if you recall how you generated them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I called head -c 4 /dev/urandom | xxd -p locally.

Instead of claiming that they are randomly generated, it may be better to generate them randomly, so that the same approach can be used for testnet5?

E.g. echo -n testnet4 | sha256sum | head -c 8, or similar, if you recall how you generated them?

The suggested way to generate them isn't random but deterministic as far as I can tell and if my definition of random isn't off. If there is a serious concern with my suggested sequence of bytes we could switch to something deterministic but making it repeatable and setting a rule in stone like suggested means the bytes for all future testnets will be known now and I am not sure this is a good thing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the only actual random byte sequence that would satisfy everyone is to take the first 4 bytes of a blockhash to which height we would previously commit to use it. If we go the deterministic route we may as well use something meaningful like the utxo set dump uses UTXO as the magic now. I can change it if we reset one more time before merging but I would like to know from more reviewers what their preference is.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, it was just a nit. I guess the exact value doesn't matter too much, except that it shouldn't collide with the bytes of the mainnet, or previous, or current testnets (including signets). I suggested a hash function, because the uniform distribution is probably the best bet at achieving the goal. (Edited out the typo in my comment, thanks).

Signet uses the hash of the challenge, so using the hash of the genesis block for testnet seems fine. (I think this is what you were suggesting in your last reply?)

In any case, just a nit and not a blocker.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It looks likely that we will reset one more time before merge of the PR so I will switch to something deterministic if we do this.

Copy link

Choose a reason for hiding this comment

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

making it repeatable and setting a rule in stone like suggested means the bytes for all future testnets will be known now

They don't have to be. For example, in Signet, you have double SHA-256 of the signet challenge. This can work in a similar way. The code for signet message start chars is already there. So, it can be identical to a signet network, where the message from the Genesis Block is set as a signet challenge. In this way, the code for generating new networks will be known upfront, but the hash of the block will remain unknown, until the new Genesis Block for the new testnet will be mined.

it shouldn't collide with the bytes of the mainnet, or previous, or current testnets (including signets)

You cannot avoid colliding with signets. Every network creator can pick its own signet challenge. And there are only four bytes. Which means, that finding a signet challenge, which will collide with any existing network, takes roughly the same effort, as mining a single block, with the minimal difficulty. Also, because of that, anyone can make a new network, and grind it to reach 0x1c163f28, and then try to connect it with testnet4.

Copy link

@joseedil joseedil left a comment

Choose a reason for hiding this comment

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

Looks good so far, considering the the comments from others.

I'm still to play a bit with the specification.

bip-testnet4.mediawiki Outdated Show resolved Hide resolved
Copy link
Member

@Sjors Sjors left a comment

Choose a reason for hiding this comment

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

Concept ACK aad54df.

I left feedback on the explanation, but the proposed rules themselves look good to me.

I'm still open to someone writing a script to produce useful test vectors, and resetting the genesis block to include those earlier.

Other than that, it seems we ran out of steam coming up with ideas to further improve testnet4 resistance against shenanigans. There's always testnet5 :-)

bip-testnet4.mediawiki Outdated Show resolved Hide resolved
bip-testnet4.mediawiki Outdated Show resolved Hide resolved
bip-testnet4.mediawiki Outdated Show resolved Hide resolved
As a result, TBTC is being actively bought and sold; one could argue that the fundamental principle of testnet coins having no value has been broken."

Since then the issue with block storms has been further demonstrated on Testnet 3 when three years' worth of blocks were mined in a few weeks while rendering the network practically unusable at the same time.

Copy link
Member

Choose a reason for hiding this comment

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

Would be nice if we can link to an earlier mailinglist, forum post or Github issue where the reset-when-worth-money policy was mentioned in the past.

Copy link
Contributor Author

@fjahr fjahr Jul 9, 2024

Choose a reason for hiding this comment

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

I did some further digging and it seems this was the trigger for the first Testnet reset but I couldn't find a good primary source for that yet. It is stated in the Wiki. Here is the PR adding it and the following reset but they don't have any details...

bip-testnet4.mediawiki Outdated Show resolved Hide resolved
bip-testnet4.mediawiki Outdated Show resolved Hide resolved
The applied changes were the result of discussions on the mailing list and the PR. The selected changes try to strike a balance between minimal changes to the network (keeping it as close to mainnet as possible) while making it more robust against attackers that try to disrupt the network. Several alternative designs were considered:

* For the block storm fix an alternative fix could have been to prevent the last block in a difficulty period from applying the existing difficulty exception. Both solutions were deemed acceptable and there was no clear preference among reviewers.
* Removal of the 20-min difficulty exception was discussed but dismissed since several reviewers insisted that it was a useful feature allowing non-standard transactions to be mined with just a CPU.
Copy link
Member

Choose a reason for hiding this comment

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

The rule is also necessary to allow the difficulty to go down faster after a miner explodes it and leaves. Rather, I think the objection was against the alternative approach of setting the minimum difficulty to an S9 miner. That would make block storms significantly more expensive, but completely break this CPU mining trick.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The removal of 20-min rule was also suggested and rejected, it's just hard to follow that because the conversation was going on in PR and ML in parallel at the time. I added another sentence on the difficulty going down, let me know if that works.


* For the block storm fix an alternative fix could have been to prevent the last block in a difficulty period from applying the existing difficulty exception. Both solutions were deemed acceptable and there was no clear preference among reviewers.
* Removal of the 20-min difficulty exception was discussed but dismissed since several reviewers insisted that it was a useful feature allowing non-standard transactions to be mined with just a CPU.
* Increase of minimum difficulty was discussed but dismissed as it would categorically prevent participation in the network using a CPU miner.
Copy link
Member

Choose a reason for hiding this comment

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

That's already impossible today on testnet4, except with the trick above - which I think is the real objection.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought that's what I am saying here. I added a hint to the 20-min exception but if that doesn't cut it for you can you suggest an alternative wording?

* Removal of the 20-min difficulty exception was discussed but dismissed since several reviewers insisted that it was a useful feature allowing non-standard transactions to be mined with just a CPU.
* Increase of minimum difficulty was discussed but dismissed as it would categorically prevent participation in the network using a CPU miner.
* Increase of the delay in the 20 min difficulty exception was suggested but did not receive significant support.
* Re-enabling <code>acceptnonstdtxn</code> in bitcoin core by default was dismissed as it had led to confusion among layer-2s that had used testnet for transaction propagation tests and expected it to behave similar to mainnet.
Copy link
Member

Choose a reason for hiding this comment

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

Standardness rules for testnet4 can be changed at a later point.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, but I don't understand what you are asking to be changed here. I am describing the discussion as the guidelines say: "It should describe alternate designs that were considered" https://github.com/bitcoin/bips/blob/master/bip-0001.mediawiki#what-belongs-in-a-successful-bip under rationale.

Copy link
Member

Choose a reason for hiding this comment

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

Whether acceptnonstdtxn is on by default in Bitcoin Core, is not part of the design of testnet4. But it's fine to just leave it here.

* Increase of minimum difficulty was discussed but dismissed as it would categorically prevent participation in the network using a CPU miner.
* Increase of the delay in the 20 min difficulty exception was suggested but did not receive significant support.
* Re-enabling <code>acceptnonstdtxn</code> in bitcoin core by default was dismissed as it had led to confusion among layer-2s that had used testnet for transaction propagation tests and expected it to behave similar to mainnet.
* Motivating miners to re-org min difficulty blocks was suggested but this may still be done after Testnet 4 is deployed, so it can be considered out of scope for this BIP.
Copy link
Member

Choose a reason for hiding this comment

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

Indeed, though it requires messing with the timestamp as explained above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't think the details how this would work exactly matter this much here. Or if you would really like something added can you suggest a different wording?

Copy link
Member

Choose a reason for hiding this comment

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

It's probably fine as-is.

@fjahr
Copy link
Contributor Author

fjahr commented Jul 9, 2024

I'm still open to someone writing a script to produce useful test vectors, and resetting the genesis block to include those earlier.

I am happy to add this but writing that script is the hard part. I am working on something but there are a lot of ideas, contributors welcome!

@fjahr fjahr force-pushed the testnet4 branch 3 times, most recently from cb202dc to 541ad43 Compare July 9, 2024 15:25
Copy link
Contributor

@murchandamus murchandamus left a comment

Choose a reason for hiding this comment

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

Formatting of the document looks good to me. I have a few suggestions on alternative phrasings and potential additions. Please feel free to take or leave as you wish.

bip-testnet4.mediawiki Outdated Show resolved Hide resolved
bip-testnet4.mediawiki Outdated Show resolved Hide resolved
bip-testnet4.mediawiki Outdated Show resolved Hide resolved

Block storms are generated by causing the nBits value of the last block to be 1, using the rule from the previous section. The multiplication factor then results in a new difficulty between 1 (since it can't be less) and 4. An arbitrarily large number of blocks can then be generated at very low difficulty. The block storm is bound only by miner hash rate, the need for every last block in the difficulty adjustment period to have a timestamp 20 minutes after the second to last block, the Median-Time-Past nTime rule and the requirement that blocks can't be more than 2 hours in the future.

A block storm does not require a time warp attack, but one can be used to amplify it.
Copy link
Contributor

Choose a reason for hiding this comment

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

It was not clear to me at first how a time warp attack could be used to exacerbate the perpetual block storm attack, but this is how I explained it to myself then:

Suggested change
A block storm does not require a time warp attack, but one can be used to amplify it.
A block storm does not require a time warp attack, but one can be used to amplify<ref>A perpetual block storm attack with entire difficulty period being authored in less than 3.5 days that resets the difficulty to the minimum in the last block of every difficulty period would adjust to a new actual difficulty of 4 every period. An attacker that additionally leverages a time warp attack would start their attack by holding back timestamps until the latest block’s timestamp is at least two weeks in the past, and then limiting their block rate to six blocks per second, incrementing the timestamp on every sixth block. Only on the last block they would use the current time, which both resets the difficulty to one per the 20-minute exception and would result in a difficulty adjustment keeping the difficulty at the minimum due to the elapsed time exceeding the target.</ref> it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have added this but maybe @Sjors can chime in if he had a different style of attack in mind.

I think what's also true and could be added here as well is that the 20-minute exception rule makes it possible to attack Testnet 4 with lower hash power than usual. I realize now that is the inverse of what is written here right now but it's been in my mind mostly when thinking of why we don't want time warp attacks.

For example, to ensure that the attacker gets to set the timestamp on the first block (or any particular block) of the difficulty period they could use the 20-min exception to reorg other miners out. They could mine in the pattern of one block with a timestamp as low as the MTP permits, then a few 20-min exception blocks on top of that to reorg anyone else who has found blocks in the meantime, then again another real one with as low as MTP permits. That should allow them to reliably keep MTP low even with low mining power compared to the rest of the network. I guess 10% of the network hashrate might be enough to exploit the time warp on Testnet 4 without timewarp protection, since you need to be able to find at least one real block within the MTP window somewhat reliably. The good thing is of course that each 20-min exception also potentially drags MTP forward so that makes the attack a bit trickier.

In general I think we should see average blocktimes faster than 10 min if the 20-min exception is taken advantage of regularly already even without doing anything to the timestamps.

I need to think a bit more about this but I can try to draft something if you think this belongs in here as well.

Copy link
Contributor

@murchandamus murchandamus Jul 19, 2024

Choose a reason for hiding this comment

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

Oh, I thought that blocks would count towards the total work of the chain according to their nBits and that would mean that even several 20-minute exception blocks would contribute less to the total work than one block mined at the actual difficulty.

I asked a clarifying question here: https://bitcoin.stackexchange.com/q/123703/5406

Edit: It sounds like you would have to mine a large number of 20-minute exception blocks to oust an actual difficulty block ;)

Copy link
Contributor

Choose a reason for hiding this comment

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

Was just thinking about this again. I am not quite sure I follow your scenario. If you were suggesting that two 20-minute exception blocks can displace one actual difficulty block, I don’t think that’s the case. The total work added by the two 20-minute blocks would only amount to difficulty 2, whereas the actual difficulty block would probably have a difficulty of more than 2.

However, if there are two actual difficulty blocks competing, one could easily decide the tie by mining a 20-minute exception block as diff_act + 1 would beat diff_act.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I guess I was exaggerating since not multiple blocks can be reorged. But I do think that what you describe in the second paragraph with the two competing blocks does lower the amount of hash power you need to successfully attack the network, though not that significantly. But it gives you a bit higher chance to find blocks more consistently since you can try to make a competing block and make it win with the 20-min block afterwards if you were not successful for a while.


A block storm does not require a time warp attack, but one can be used to amplify it.

The mitigation consists of adding a new rule, to no longer apply the adjustment factor to the last block of the previous difficulty period when its difficulty is 1. In that case blocks are traversed in reverse order until a block is found with a higher difficulty. If the first block of the previous period also has difficulty 1 than this is used.
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need to traverse in reverse order at all? Wouldn’t it be easier to always use the first block’s nBits to calculate the difficulty adjustment? It seems to me that it would even be compatible with mainnet.

Suggested change
The mitigation consists of adding a new rule, to no longer apply the adjustment factor to the last block of the previous difficulty period when its difficulty is 1. In that case blocks are traversed in reverse order until a block is found with a higher difficulty. If the first block of the previous period also has difficulty 1 than this is used.
The mitigation consists of adding a new rule, to no longer apply the adjustment factor to the last block of the previous difficulty period when its difficulty is 1. In that case, blocks are traversed in reverse order until a block is found with a higher difficulty. Only if all blocks of the previous period have a difficulty of 1, including its first block, difficulty 1 is used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right, that makes sense. I think the traversing back just made sense to me initially because that's what we are also doing when we look for the real difficulty of a normal 20-minute exception block. I will look into simplifying the code in the PR with this in mind and then update here.

Also took the suggested change here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have applied that change to the PR now and also made the respective edit here.

bip-testnet4.mediawiki Outdated Show resolved Hide resolved
bip-testnet4.mediawiki Outdated Show resolved Hide resolved
bip-testnet4.mediawiki Outdated Show resolved Hide resolved
bip-testnet4.mediawiki Outdated Show resolved Hide resolved
bip-testnet4.mediawiki Outdated Show resolved Hide resolved
Copy link
Contributor

@murchandamus murchandamus left a comment

Choose a reason for hiding this comment

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

Oh, I noticed that the document uses "20 min difficulty exception", "20-min exception", "20-min difficulty exception", and "20-min rule" for the same concept. I would recommend to use one term in all instances: "20-minute exception".

bip-testnet4.mediawiki Outdated Show resolved Hide resolved
bip-testnet4.mediawiki Outdated Show resolved Hide resolved
bip-testnet4.mediawiki Outdated Show resolved Hide resolved
bip-testnet4.mediawiki Outdated Show resolved Hide resolved
Copy link
Contributor

@murchandamus murchandamus left a comment

Choose a reason for hiding this comment

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

Gave it another read. Looks good to me, got a couple more suggestions.

bip-testnet4.mediawiki Outdated Show resolved Hide resolved
bip-testnet4.mediawiki Outdated Show resolved Hide resolved
bip-testnet4.mediawiki Outdated Show resolved Hide resolved
* Increase of the delay in the 20-minute exception was suggested but did not receive significant support.
* Re-enabling <code>acceptnonstdtxn</code> in bitcoin core by default was dismissed as it had led to confusion among layer-2s that had used testnet for transaction propagation tests and expected it to behave similar to mainnet.
* Motivating miners to re-org min difficulty blocks was suggested, but was considered out of scope for this BIP, since adoption of such a mining policy remains available after Testnet 4 is deployed.
* Persisting the real difficulty in the version field was suggested to robustly prevent exploits of the 20-minute exception while allowing it to be used on any block, but did not receive a sufficient level of support to justify the more invasive change.
Copy link
Contributor

Choose a reason for hiding this comment

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

Also, that happens essentially in the first block of each difficulty period automatically without introducing any other changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm, I am not sure if that is relevant. The idea of this was to make it possible to make the first block also a 20-min block so the "other changes" are implied in this. And those basically are also what is referred to as the invasive change.

@murchandamus murchandamus removed the PR Author action required Needs updates, has unaddressed review comments, or is otherwise waiting for PR author label Jul 22, 2024
@murchandamus
Copy link
Contributor

Let’s call this BIP 94.

Could you please add the number to the PR title, put it into the preamble, and insert a table entry in the README for your BIP?

@RandyMcMillan
Copy link
Contributor

@maflcko

To prevent a premine - take the bits from a hash of a future mainnet block:

psuedo code
'''
if mainnet_blockheight >= 898898
then
messageStart = sha256(mainnetblock_hash_898898)
...
'''

This method can be reused for all future testnets?

This is both deterministic and verifiable?

@RandyMcMillan
Copy link
Contributor

@maflcko

To prevent a premine - take the bits from a hash of a future mainnet block:

psuedo code
'''
if mainnet_blockheight >= 898898
then
messageStart = sha256(mainnetblock_hash_898898)
...
'''

This method can be reused for all future testnets?

This is both deterministic and verifiable?

Couldn't the mainnet_blockheight be a parameter - enabling devs to spin up their own testnet4? It may be useful to miners for testing? In fact - every future block can seed its own testnet4. :)

@RandyMcMillan
Copy link
Contributor

RandyMcMillan commented Jul 22, 2024

A parameterized testnet4 <mainnet_blockheight> may have the effect of allowing inscribers to use a testnet4 at mainnet_blockheight <int> for their own purposes - mitigating the incentives to pollute mainnet as well as the global testnet4 at blockheight <tbd>

@RandyMcMillan
Copy link
Contributor

Note: I was - in jest - calling this "blobnet" :)

https://x.com/randymcmillan/status/1620117331010543624?s=46

@jonatack jonatack changed the title Add BIP Testnet 4 BIP94 Testnet 4 Jul 24, 2024
@jonatack jonatack added the PR Author action required Needs updates, has unaddressed review comments, or is otherwise waiting for PR author label Jul 24, 2024
@fjahr fjahr force-pushed the testnet4 branch 2 times, most recently from 3bfbe70 to ebc9b17 Compare July 30, 2024 22:11
@fjahr
Copy link
Contributor Author

fjahr commented Jul 30, 2024

A parameterized testnet4 <mainnet_blockheight> may have the effect of allowing inscribers to use a testnet4 at mainnet_blockheight for their own purposes - mitigating the incentives to pollute mainnet as well as the global testnet4 at blockheight

Part of what makes Testnet still interesting is that there are lots of others using it. So I think this is a different feature that is out of scope for this BIP, requiring seperate conceptual review etc..

Copy link
Contributor

@murchandamus murchandamus left a comment

Choose a reason for hiding this comment

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

The second CI check fails because of the Comments-URI line, but I’m not sure why the third one fails. Perhaps it will be fixed by the Comments-URI as well.

bip-0094.mediawiki Outdated Show resolved Hide resolved
bip-0094.mediawiki Outdated Show resolved Hide resolved
Copy link
Contributor

@murchandamus murchandamus left a comment

Choose a reason for hiding this comment

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

ACK 451f453

Left a few nits, but it also seems fine to be merged as is. What’s the plan regarding the Genesis Block? I seem to recall that we might reset another time before rolling it out for good.


=== Block Storm Fix ===

The work required for a new difficulty period is calculated as multiplication factor to the difficulty of the previous period (but no less than 1/4th and no more than 4x). On Mainnet and Testnet 3, this factor is applied to the nBits value of the last block.
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: nBits is the target, and the difficulty is inversely related to the target, so the factor that is being applied to the difficulty is the inverse of the factor being applied to nBits.

I also notice that it is not mentioned that the factor is based on the duration of the prior difficulty period.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Make some edits here that should address this.

bip-0094.mediawiki Outdated Show resolved Hide resolved
bip-0094.mediawiki Outdated Show resolved Hide resolved
bip-0094.mediawiki Show resolved Hide resolved
@fjahr
Copy link
Contributor Author

fjahr commented Jul 31, 2024

What’s the plan regarding the Genesis Block? I seem to recall that we might reset another time before rolling it out for good.

I will bring this up in the Bitcoin Core IRC meeting tomorrow. It may not be the perfect venue but I am not sure what could be a better one and I would like to know what we do before v28 feature freeze. Opinions mentioned to me have been split throughout the last months but I think there is more upside to not resetting. I don’t see an issue with the “pre-mine” of 40k blocks since many of these coins seem to be available through free faucets right now which makes it easier for anyone to get some right from the start. There currently is some distribution of these coins among several miners and it’s not clear to me that a more public re-launch next week gives us a fairer result. Maybe someone invests some real hashrate in order to get all of those first 40k blocks. And not resetting allows us to also set a min chain work as @Sjors mentioned on the PR. So I see more upside on not resetting personally but happy to discuss it.

If we had gotten the task done of embedding scripts then we could have done the reset with that but I don't think I will be able to finish that work by next week and nobody else seems to have stepped up either. I am still going to try to add these scripts in the later parts of the chain and make the tooling available for an eventual Testnet 5.

@murchandamus
Copy link
Contributor

Okay, then let’s reconvene after Bitcoin Core meeting and consider whether we should merge. Thanks for the quick turnaround on my review.

@murchandamus murchandamus removed the PR Author action required Needs updates, has unaddressed review comments, or is otherwise waiting for PR author label Jul 31, 2024
@fjahr
Copy link
Contributor Author

fjahr commented Aug 1, 2024

Based on the feedback from the IRC discussion today I would say this is ready for merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet