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

Allow perpetual dft mining to correctly accept the next bitwork #110

Merged
merged 10 commits into from
Feb 2, 2024

Conversation

atomicals
Copy link
Owner

@atomicals atomicals commented Jan 31, 2024

Problem

Users who mine the next bitwork difficulty instead of the current will not be awarded the dft mint. This problem is magnified due to the atomicals-js CLI as of the previous version choosing to mine the next bitwork difficulty for an infinitely mineable token.

The user impact is that the users who attempted to mine the next difficulty after a certain threshold will not get their token mint because the indexer does not recognize bitwork difficulties higher than the currently acceptable one.

For example, if the current bitwork is 7777.15 and the next bitwork is 77777 then anyone who mined 77777 will not be accepted because according to the current rules (before this fix) only 7777f is valid (due to the presence of 15).

The solution below focuses on also allowing the 77777 to be mined along with the 7777f since they are both technically the same difficulty.

Solution

The solution presented here is to allow the indexer flexibility to also accept the next difficulty gracefully and award the user their dft mint.

The solution is activated (param ATOMICALS_ACTIVATION_HEIGHT_BITWORKEXT) on livenet as of height 828500 and testnet as of height 2576412.

When the new configuration becomes active, a Bitwork will be accepted from the next increment roll over.

Unit tests:

Note: Care must be taken for roll overs with increments that are not necessarily evenly divided by 16.
Run tests with: pytest ./tests/lib/test_atomicals_utils.py

https://github.com/atomicals/atomicals-electrumx/blob/accept-next-bitwork/tests/lib/test_atomicals_utils.py#L463

@atomicals atomicals force-pushed the accept-next-bitwork branch from ae99739 to 84608d9 Compare January 31, 2024 18:41
Copy link
Contributor

@AlexV525 AlexV525 left a comment

Choose a reason for hiding this comment

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

The idea is good overall. Thanks for the patch which significantly helps users to save their mints!

To prove the next difficulty can be equal to the current, we might make tests to iterate the combination of inc and start to find out if we can avoid anti-patterns as much as possible.

@@ -459,3 +460,116 @@ def test_calculate_expected_bitwork_base():
assert(calculate_expected_bitwork('abcdefe', 33000, 1000, 1, 127) == 'abcdefe000')
assert(calculate_expected_bitwork('abcdefe', 33000, 1000, 3, 127) == 'abcdefe0000000.2')

def test_calculate_expected_bitwork_rollover():

assert(calculate_expected_bitwork('888888888888', 49995, 3333, 1, 64) == '8888.15')
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider adding cases to guard increments other than 1.

Choose a reason for hiding this comment

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

please also consider adding cases for more general cases like: 8888.2, 8888.3, 8888.8, 8888.9

Copy link
Owner Author

Choose a reason for hiding this comment

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

Done, added a variety and using an array-loop

@@ -1664,6 +1663,21 @@ def get_subname_request_candidate_status(current_height, atomical_info, status,
'pending_candidate_atomical_id': candidate_id_compact
}

# Whether txid is valid for the current and next bitwork
def is_txid_valid_for_bitwork(txid, bitwork_vec, actual_mints, max_mints, target_increment, starting_target, allow_next):
Copy link
Contributor

Choose a reason for hiding this comment

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

For readability: rename to is_txid_valid_for_perpetual_bitwork

Copy link
Owner Author

Choose a reason for hiding this comment

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

Done

return None

mint_bitworkr_start = mint_info_for_ticker.get('$mint_bitworkr_start')
if height >= self.coin.ATOMICALS_ACTIVATION_HEIGHT_BITWORKEXT:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Get error AttributeError: type object 'BitcoinTestnet' has no attribute 'ATOMICALS_ACTIVATION_HEIGHT_BITWORKEXT'

Copy link
Owner Author

Choose a reason for hiding this comment

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

Done

@jiedo
Copy link

jiedo commented Feb 1, 2024

Please don't just fix the problem for the cli mining mode, we need to make the txid that already meets the longer bv prefix valid under any shorter bitwork requirements with ext.

return None

mint_bitworkr_start = mint_info_for_ticker.get('$mint_bitworkr_start')
if height >= self.coin.ATOMICALS_ACTIVATION_HEIGHT_BITWORKEXT:
Copy link

@NeutronProtocol NeutronProtocol Feb 1, 2024

Choose a reason for hiding this comment

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

nit: this is essentially

Suggested change
if height >= self.coin.ATOMICALS_ACTIVATION_HEIGHT_BITWORKEXT:
if self.is_dft_bitwork_rollover_activated(height):

Copy link
Owner Author

Choose a reason for hiding this comment

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

Done

@shadowv0vshadow
Copy link
Collaborator

Fully test, it work, here is my test log

txid: 17fc218b106cad775c2a3a8a47c9ef650deaf4f29bd89007b61625c16329bb17
max_mints_global: None
mint_bitwork_vec: 777777777777
mint_bitworkc_inc: 1
mint_bitworkr_inc: 0
height 2576474
mint_bitworkc_start: 64
bitwork_str: 7777.15
success: True
txid: cccb1574a69913de166b68bccd05f079565a1a9c43981d91c702c202538b21b3
max_mints_global: None
mint_bitwork_vec: 777777777777
mint_bitworkc_inc: 1
mint_bitworkr_inc: 0
height 2576474
mint_bitworkc_start: 64
bitwork_str: None
success: False
txid: d783b6c804ea0cff00ced96cc199f01d7ccba5fed282d7560d42e7c46881a37c
max_mints_global: None
mint_bitwork_vec: 777777777777
mint_bitworkc_inc: 1
mint_bitworkr_inc: 0
height 2576477
mint_bitworkc_start: 64
bitwork_str: 77777
success: True
perpetual
txid: dc5ece2dd7b248d289aa4376019165b2a21880d10ed6cbb53ae97cdbba0cb7de
max_mints_global: None
mint_bitwork_vec: 777777777777
mint_bitworkc_inc: 1
mint_bitworkr_inc: 0
height 2576477
mint_bitworkc_start: 64
bitwork_str: 77777
success: True
perpetual
txid: 815ac388f2cb856c01c6aad4dd56656b948bc71872c2f103bb565bad75c2e2f3
max_mints_global: None
mint_bitwork_vec: 777777777777
mint_bitworkc_inc: 1
mint_bitworkr_inc: 0
height 2576478
mint_bitworkc_start: 64
bitwork_str: 77777
success: True

The only failed tx's bitwork is 7777b, Indexer accpeted the 77777 when mint_bitworkc_current equal 7777.15.

@atomicals
Copy link
Owner Author

Please review again as there have been some changes and additional tests to accept the next full complete bitwork string.

ie: 88888 should be valid for 8888.10

base_bitwork_padded = bitwork_vec.ljust(32, '0')
b = base_bitwork_padded[:current_prefix_len]
p = base_bitwork_padded[:current_prefix_len + 1]
print(f'next_full_bitwork_prefix b={b} p={p} ')
Copy link
Collaborator

Choose a reason for hiding this comment

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

No need print, logger.debug?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Done and removed

mint_bitworkr_start = mint_info_for_ticker.get('$mint_bitworkr_start')
if height >= self.coin.ATOMICALS_ACTIVATION_HEIGHT_BITWORKEXT:
if not is_txid_valid_for_perpetual_bitwork(atomicals_operations_found_at_inputs['commit_txid'], mint_bitwork_vec, decentralized_mints, max_mints, mint_bitworkr_inc, mint_bitworkr_start, True):
self.logger.warning(f'create_or_delete_decentralized_mint_output: mint_bitworkc_inc not is_mint_pow_valid {hash_to_hex_str(tx_hash)}, atomicals_operations_found_at_inputs={atomicals_operations_found_at_inputs}...')
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
self.logger.warning(f'create_or_delete_decentralized_mint_output: mint_bitworkc_inc not is_mint_pow_valid {hash_to_hex_str(tx_hash)}, atomicals_operations_found_at_inputs={atomicals_operations_found_at_inputs}...')
self.logger.warning(f'create_or_delete_decentralized_mint_output: mint_bitworkr_inc not is_mint_pow_valid {hash_to_hex_str(tx_hash)}, atomicals_operations_found_at_inputs={atomicals_operations_found_at_inputs}...')

Copy link
Owner Author

Choose a reason for hiding this comment

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

Done

return None
else:
if not is_txid_valid_for_perpetual_bitwork(atomicals_operations_found_at_inputs['reveal_location_txid'], mint_bitwork_vec, decentralized_mints, max_mints, mint_bitworkr_inc, mint_bitworkr_start, False):
self.logger.warning(f'create_or_delete_decentralized_mint_output: mint_bitworkc_inc not is_mint_pow_valid {hash_to_hex_str(tx_hash)}, atomicals_operations_found_at_inputs={atomicals_operations_found_at_inputs}...')
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
self.logger.warning(f'create_or_delete_decentralized_mint_output: mint_bitworkc_inc not is_mint_pow_valid {hash_to_hex_str(tx_hash)}, atomicals_operations_found_at_inputs={atomicals_operations_found_at_inputs}...')
self.logger.warning(f'create_or_delete_decentralized_mint_output: mint_bitworkr_inc not is_mint_pow_valid {hash_to_hex_str(tx_hash)}, atomicals_operations_found_at_inputs={atomicals_operations_found_at_inputs}...')

Copy link
Owner Author

Choose a reason for hiding this comment

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

Done


mint_bitworkr_start = mint_info_for_ticker.get('$mint_bitworkr_start')
if height >= self.coin.ATOMICALS_ACTIVATION_HEIGHT_BITWORKEXT:
if not is_txid_valid_for_perpetual_bitwork(atomicals_operations_found_at_inputs['commit_txid'], mint_bitwork_vec, decentralized_mints, max_mints, mint_bitworkr_inc, mint_bitworkr_start, True):
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be reveal_location_txid?

Suggested change
if not is_txid_valid_for_perpetual_bitwork(atomicals_operations_found_at_inputs['commit_txid'], mint_bitwork_vec, decentralized_mints, max_mints, mint_bitworkr_inc, mint_bitworkr_start, True):
if not is_txid_valid_for_perpetual_bitwork(atomicals_operations_found_at_inputs['reveal_location_txid'], mint_bitwork_vec, decentralized_mints, max_mints, mint_bitworkr_inc, mint_bitworkr_start, True):

Copy link
Collaborator

Choose a reason for hiding this comment

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

ATOMICALS_ACTIVATION_HEIGHT_BITWORKEXT not define, need fix

Copy link
Owner Author

Choose a reason for hiding this comment

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

Done

@atomicals atomicals force-pushed the accept-next-bitwork branch from a73ec52 to 077d276 Compare February 1, 2024 19:34
@atomicals atomicals force-pushed the accept-next-bitwork branch from bfc74e2 to b3c5a5e Compare February 1, 2024 23:22
@atomicals atomicals merged commit 2372c56 into master Feb 2, 2024
@AlexV525
Copy link
Contributor

AlexV525 commented Feb 2, 2024

FYI to everyone: the activation height is 828628 instead of 828500.

ATOMICALS_ACTIVATION_HEIGHT_DFT_BITWORK_ROLLOVER = 828628

@atomicals atomicals deleted the accept-next-bitwork branch April 10, 2024 00:38
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.

5 participants