What Changes Were Included in the Creation of Hive (Hard Fork 23)?

in #hive5 years ago

You can see all the code changes here:
https://github.com/steemit/steem/pull/3626/files

All the commits here:
https://github.com/steemit/steem/pull/3626

There are 51 total commits including one from back in September which start March on 16th. I'll go through each one (minus merge commits) and explain them as I understood them when I did my review of the code as a witness before deciding to run v0.23.0. I should point out that I'm not a C++ developer, and I did not write any of this code. Full credit goes to those who worked tirelessly on this effort. As a witness with a development background, I always review code before deploying it and encourage all witnesses to do so or hire someone you trust to do so for you (ideally not an existing consensus witness).

Most of the commits have very descriptive commit messages which do a fairly good job of explaining what's going on. As far as I can tell, it doesn't appear anything malicious was included in this hard fork.

  • 0c5da8e7016739befceb99136d9237ef43e4facf


    [BW]: Changed STEEM and SBD coin names in the legacy_asset implementation.
    [BW]: Renamed get_config parameter names.
    [BW]: Eliminated STEEM and SBD names from assertion messages etc.

    Pretty straight forward. Just renames token symbols.

  • 74177d3a7c3a96a7f7e007195e1f5468f0daa9cd


    Remove all delegations and steem/sp/sbd from steemit accounts

    Adds STEEM_HARDFORK_0_23 and starts looping through accounts defined here:

    for (auto account_name : hardforkprotect::get_steemit_accounts()) {

    image.png

    My understanding of this code includes, for specific accounts, moving their stake to the Hive Proposal treasury account which, as you can see here, is not controlled by anyone but the blockchain code itself (it has no owner or active keys):

    image.png

    This code removes proxy votes, delegations, adjust voting mana, and ultimate adjusts token balances.

    The code also prepares things to set a date for the Hard Fork both on the testnet and the mainnet.

  • 1bcf56a7d50f694007d04ad2be9a53f57cb1e2ee


    transfer escrow, savings to dao. remove pending curation and posting rewards

    More transfers of impacted account funds to the treasury. This included things most people probably don't think about such as escrows and savings balances.

  • 9840b8315c11d2c08d501955d41fd186a04cd592


    Added support for rewards along with global supply

    As far as I know, this puts pending rewards back into the global supply.

  • e9f2f7302bbf691c8084383edc85d10f4aee58f9


    hardfork vop

    I think this adds a virtual operation hardfork_hive_operation to actually implement the hard fork changes which move balances around.

  • 1b421e7981f0844dd559790b40f7c508d2d6981b


    [BW]: Added possibility to specify hardfork time through env. variable to make testing easier.
    [BW]: Witnesses version majority rules are excluded for HF23

    Helps with testing to set the hard fork time to be an environment variable so you don't have to recompile the code each time to test your hard fork changes. As for the second part, that's a bit more tricky.

    ilog("Forcing HF23 without need to have witness majority version");
    

    I asked Blocktrades about this when I was reviewing the code and he gave me permission to share that discussion with you:

    image.png
    image.png

    (Note, I've since decided not to run a witness node on Steem)

    This is a little tricky, but as far as I can tell it allows a non-majority group of witnesses (as is the case when the chain is being sybil attacked by 20 sock puppet witnesses so community-supported witnesses are not the majority) to create their own valid fork for this chain id and keep running with it on their own to implement HF 23. For more on the philosophy behind this, see @blocktrades post The Fundamental Underpinning of Blockchain Consensus

  • 39db349c7f76c58bc7d54ec46dae6b7669117f8d


    [BW]: [NIP] Implemented storage for transaction_id inxed in AH-rocksdb version. Also implemented API query to execute get_transaction.

    I think this allows for improvements to account history so individual transaction ids can be indexed and retrieved with find_transaction_info so instead of

    FC_ASSERT( false, "This API is not supported for account history backed by RocksDB" );
    

    We get code to use RocksDB and full history. That's my understanding, at least.

  • 0d662f1bc49dc0e16cf982a19be7059ed5e7c5c9


    Added escrows to as target as well and fixed vests withdrawal issues

    Looks to be some code fixes relating to escrow and how vested shares are handled on withdrawals.

  • 37b43569ecd1ece086e3c594573e80dfa828ce10


    STEEM and SBD rename supplement:

    Dump method added for debugging along with some more descriptive information if the database is created by a different compiler, build, or operating system. Asset symbols HBD_SYMBOL_U64 and HIVE_SYMBOL_U64 added to work along side SBD_SYMBOL_U64 and STEEM_SYMBOL_U64

  • a368be40a7da294a63c28c1b45309a8413cd0317


    [ABW]: [Fix] wrong balance affected when removing savings

    minor fix, looks like a copy/paste error.

  • a341ce4d3fc08fe78288c0147f8247f42b574718


    [ABW]: [Fix] it should already be at zero and if it wasn't making it zero like that would break invariants

    minor fix, initializing variables

  • 9a3a42d2dcecf1b5944e57eafb538098a7c5a089


    [ABW]: [Fix] reward_vesting_steem is STEEM, not VESTS; also duplicate code

    Appears to be a minor fix as it already has:

                   a.reward_vesting_balance = asset( 0, VESTS_SYMBOL );
                   a.reward_vesting_steem = asset( 0, STEEM_SYMBOL );
    
  • 074bd5336beb0890ae17c4dc0adf4bb558dddab8


    [ABW]: [Fix] no need to break historical record of the account

    Looks like without this fix it would have reset curation and posting rewards history to zero. Glad this was fixed so that Hard Fork 23 is more of a continuation of Steem.

  • 3d096d9173bb013c97d41935e713285374f118c6


    [ABW]: [Fix] looking for 'to' would require different index that does not exist now (plus we don't need it); missing handling of pending_fee would break invariants

    Looks to be some more fixes for handling escrow balances.

  • eeabcc71f6192e4cf10a38e8c065c1b6f83325fa


    [ABW]: [Fix] missing clearing of open limit orders

    Ensures if the accounts being cleared out had open orders on the internal market, those orders get cancelled.

  • 1a84fdc541af72e65260b76926cd5fe78b4cb148


    [ABW]: [Fix] missing clearing of pending convert requests

    Same as above but for pending HBD conversions to HIVE and calls adjust_balance as needed to put the HBD back in the account.

  • 6b4a2aa3c74bdd84aebf27cb10cc7b8a89c054d7


    [ABW]: [Ign] minor tweak

  • 24d2be491e715520d1d56dd6125e5e30bed8719e


    [ABW]: [Fix] gpo.total_vesting_shares and gpo.total_vesting_fund_steem (almost) always change in tandem, and always in the same direction

    I'm not entirely sure why it was + before, but this puts it back to a - to update total_vesting_fund_steem just like total_vesting_shares.

  • 3a7d4809ae4b7a09f1f6ab37b2965a0b9a1d7e5f


    [ABW]: [Fix] mix of claim_reward_balance_operation and conversion from vests to steem cancel out their influence on gpo.total_*; also avoid actual conversion back and forth

    I'm not entirely sure, but I think this is some fixes to where it converts Steem Power into liquid STEEM for the effected accounts.

  • 64d6e880f81f278b61b0a611c99edb554c966883


    [ABW]: [Fix] unused variable and related code removed; unnecessary operation removed from inside loop (it only needs to be done once before clearing)

    Relates to downvote pool clearing.

  • d1aba596c87c02a2a3405e64f0c3a084405cbe9c


    [ABW]: [Fix] wrong index used - needs two separate runs for 'from' and 'to' to cancel outgoing withdrawals and speedup incoming ones (resulting balance is taken out later)

    Related to savings account withdrawals.

  • b6f4d1fb9a9d6197f0726d716ed127669a67871d


    [ABW]: [Ign] formatting

  • 929b67da33bedb1f8a9c3179ff9a65a2f7346069


    [ABW]: [Fix] missing handling of expired delegation objects; also loop on delegations simplified

    Looks like some code refactoring along with ensure expired delegations are properly managed in the hard fork.

  • d0bc5c2ca6f09642b95209bc6750c27f35a4e7eb


    Reset TAPOS buffer to avoid replay attack

    image.png

    I think this is an important one to ensure both chains operate independently even though they have the same chain id and to ensure someone could not do a double spend right as the fork was happening. TAPOS stands for transaction as proof of stake.

  • 3c761638526bc1d45a9171e24029b140ef2bf489


    init default values for SBD asset with proper symbol

    Minor initialization fix.

  • 713fdc6f46407b875406520f2f95ac41f4e745b4


    [ABW]: [Ign] code for HF23 moved to separate routine for easier testing

    Code refactoring

  • 60e81a3dcc61f5fec59fcebd9057e1e5b101766b


    First HF unit-test - doesn't work

    Testing, hurray!

  • d01fc9d181f67adc58b256a0ef0f687fbc3f2737


    [BW]: HF23 time can be specified via env. variable also for MainNet
    [BW]: Changing default HF23 time to quite far future (April 8.)

    More tweaks to the HF time.

  • 03478f6c446815edcbee8ef3d37c5cb60580d223


    The simplest unit-test works

    More testing, more hurray!

  • 3cb9b67e767b1fec19f7480cca77c6ddb0c08639


    Voting + delegations - unit-tests part 1

    Even more testing, even more hurray!

  • 8b2ba15160f6985f13995abce70993caf944927d


    Voting + delegations - unit-tests part 2

    We like tests! Tests keep blockchains from breaking. :)

  • 5027f29ee6ee4b917775123b683c23ed6a16b1dc


    [BW]: [HF23 time set to Friday, 20 March 2020 14:00:00 GMT

  • 98ff1a584b372dcf289994f8b5bbc24783c1d0f4


    [BW]: Fixed compile failure in TEST_NET build

  • ad3414d7d02dc4d570165e65051a81d4906bb49a


    Signal HF23 sooner

    This lets people show they are moving to 0.23.0 prior to the hard fork time.

  • 4fe70ee52ada52bca99cd72fa87eca73553b6f3b


    changed Steem Network to Hive NW and added ASCII logo

    Updating the start up messages with some cool ascii.

    image.png

  • fcf74915950b4546a7abca1836ddf4134e7bb5db


    Exclude accounts who voted a minimum of two sockpuppets and who didn't unvote before the hive announcement with more than 1k sp

    This is where the account list was updated based on a script that was run. There have been reports of errors with how this list was calculated which is what I believe the dev team will be working on next to resolve. If you were negatively impacted by this, please reach out to the developers involved.

  • 2b8f66f5b49b3a446b386ca4abf1e0ff88d06483


    remove last comma

    Minor tweak to account list

  • 86ec9f1716744cf708b703981834e83428d447cb


    added sockpuppets

    This adds the Steemit controlled sock puppet accounts.


And that's it!

I don't know how helpful this was for most people as everything is right there on GitHub anyway, but hopefully it's useful to some to know there are witnesses who look over all code changes in detail before deploying them. To me, this is a base requirement for a witness, otherwise they are running a protocol version that entirely trusts whoever developed the code and aren't really adding their own individual validation to the process.


Luke Stokes is the Managing Director for the Foundation for Interwallet Operability as well as the Interim Executive Director for the EOS Foundation. He's passionate about voluntary systems of governance and has been involved in bitcoin since early 2013. He's been a witness for the Steem blockchain since early 2018 and a custodian for eosDAC, a community-owned EOSIO Block Producer and DAC Enabler, since its inception. With a computer science degree from UPENN, he built, bootstrapped and co-founded the shopping cart software company FoxyCart over a ten year period and is now focused on blockchain technology as a means to create a world we all want to live in. He currently lives in Puerto Rico with his wife and three children and enjoys discussing everything from philosophy, to consciousness, to voluntaryism, to love and awakening. lukestokes.info UnderstandingBlockchainFreedom.com fio.foundation eosdac.io

I'm a Witness! Please vote for @lukestokes.mhth

Sort:  

You mean they didn't just beep boop beep a new hardfork into existence?

Actual work was done here?

Maybe Ned and Justin should take a lesson.

:D

Yes, real coding and not just j...ing off in public with a lots of bots cheering you on.

Appreciating the transparency and accountability - and especially appreciating the non-nerd translation. 😊 Thank you.

I think it's helpful. I wouldn't have looked at the list of changes if not for your post.

Good break down IMO.

J

Hi Luke,
I just want to check my understanding regarding two point that may well be misconceptions on my part.

  1. Only one third of the top twenty witnesses need to agree in order to make a rule change
  2. The Hive tokens that would have gone to Steemit account holders had they been included in the fork, will go into a fund for development.

Am i understand this correctly?

No, DPoS requires 2/3+1 majority for a rule change. The tweaks here were designed to allow a non-majority group to create a new chain which is what happened. On that new chain, the 2/3+1 DPoS rule is still in place (unless they wanted to create a new chain, etc).

For #2, it only impacts the accounts listed in the code there, but yes, the tokens were not burnt, they were sent to the proposal system which an be used for whatever the token holders agree to (such as development).

Thanks for clearing that up form me.
And I am guessing that the J Sun Stake is being handled the same way?

Nice code review.

Hello ,
if i want to be a witness where can i find the latest software

Next time someone forks Steem or Hive, they should not airdrop any dollars at all. "Hind sight is always 20/20" but the Hive price is on its back. And like a turtle, once you get it on its back, it's forked.

I have nothing to do with @spaminator. I don't know what is a mistake and what is deliberate. I have nothing to do with @buildawhale.

Downvotes are not censorship, they are how the blockchain functions. Even fully downvoted articles on are no 0 visability on interfaces like Peakd. What is censorship is what Steemit has done which is to remove content from their hivemind API such as all my posts on the my @lukestokes.mhth account.

You're saying things that simply are not true.

Why are you tagging all these accounts? Talk to token holders about their witness votes if you don't agree with who they are voting for.

I am building up another competing condenser. Right now, it's Steem and not Hive. https://www.steemfiles.com .