FactomProject / factomd

Factom Daemon
https://www.factomprotocol.org/
Other
201 stars 92 forks source link

Develop + Wax merge #1056

Closed WhoSoup closed 3 years ago

WhoSoup commented 4 years ago

I think I managed to make a halfway decent merge, though this is not without issues. Whenever there was a conflict I couldn't quickly fix, I made a comment with the text WAX MERGE and a short explainer of what happened or what needs to be done.

  1. By far the biggest conflict is the event service repo. In Wax, the /events folder was moved to /modules/livefeed, with a new /modules/events package that is supposed to handle emitters. Meanwhile in develop, the /events package received several updates. I adjusted some of it but commented out a bunch that needs to be adapted.
  2. The second biggest source of conflicts was a lot of bugfix code by clay roughly 10 months ago. I manually compared the changes and applied what I thought made more sense, which usually meant adopting clay's bugfix code in wax.

Other than that, everything was fairly straightforward. My method of creating this patch was:

  1. Checkout develop
  2. Merge FD-1225_wax_rollup into develop
  3. Resolve all merge conflicts, favoring adopting the wax changes
  4. Run a diff between the new branch and FD-1225_wax_rollup to see what changed
  5. Manually look at all the differences in 4 and decide on a resolution

Doing it this way has the drawback of missing things that were added in develop, such as the changes to the livefeed code. These changes can be found by doing a diff of the new branch with develop, but this unfortunately has all of the Wax changes listed, making it too much work. The other option would be to find out the commit that FD-1225_wax_rollup diverges from master, then do a diff between that commit and develop, look at all the differences, and see if they are missing in the merge. This is also potentially a lot of work but might be easier than the other method.

This branch currently builds but most likely does not pass unit tests yet.

WhoSoup commented 4 years ago

Finally managed to track down the simulation bug.

Turns out that wax changed state.RequestTimeout from int to time.Duration: https://github.com/FactomProject/factomd/blob/b13c38fdf9d0e55de2f000325a86a80ae33d7237/state/state.go#L226 vs https://github.com/FactomProject/factomd/blob/10afe186954161e3831064b41856cb4385fea3f0/state/state.go#L99

Which meant that a fix in develop that tackled a bug in Catchup was now using an unexpected variable type: https://github.com/FactomProject/factomd/blob/b13c38fdf9d0e55de2f000325a86a80ae33d7237/state/dbStateCatchup.go#L46

This in turn meant that RequestTimeout was now 30 seconds (30,000,000,000 nanoseconds) rather than just plain "30" and the timeout was calculated to be around 238 years.

WhoSoup commented 3 years ago

I decided to go through the other way, too. Wax is based on commit 85932f54d0f8ad1aa89e3d5bdbd0999a2f358017, so I compared every change between that commit and develop. It looks daunting at first but approximately 95% of it is just commentary and the CheckNilHash/Logging change. Most of the remaining changes managed to merge successfully in the original merge.

Here's a brief overview in shorthand (I marked them "ok" as I checked them off/fixed them):

ok circle config changes
ok AUTHRORITY_SET_MAX_DELTA in activationHeight
    DBStateMsg.Validate
    FedVoteLevelMsg.ElectionProcess
    TimeoutInternal.ElectionProcess
    election/elections.go IsSafeToReplaceFed
    ProcessAddServer
    ProcessRemoveServer

ok primitives.CheckNil introduction
ok more checkpoints in common/constants
ok MaxEntrySizeInBytes constant
ok EC_CHAINID_STRING constant
ok PreBootWindow constant
ok Ethereum Anchoring
ok NewCommitEntry(), add Init()
ok IMsg GetReceivedTime/set
ok state.IsRunLeader
ok state.FactomSecond
ok ack.Validate, drop future acks 
ok dbstate_test.go TestDBStateValidateReplaceFeds
ok AuthorityListInternal faked message hash
ok EomSigInternal, change MessageHash to MessageHash()
ok RemoveAuditInternal, change MessageHash to MessageHash()
ok RemoveLeaderInternal, change MessageHash to MessageHash()
ok TimeoutInternal, change MessageHash to MessageHash()
? factomd/log/ folder
    remove log/log.go, rework log/logger.go
ok MissingData.FollowerExecute moved to goroutine in NPN
ok MessageBase.SendOut built in pokemon check
ok msgsupport/general.go init func
ok primitives/hash.go
    CheckNil
    Loghashfixed err!=nil bug

* live feed
    interfaces.IDBState 
    ProcessDirBlockInfoBatch EmitDirectoryBlockAnchorEvent
    ProcessDirBlockInfoMultiBatch EmitDirectoryBlockAnchorEvent
    databaseOverlay/overlay.go parentState, NewOverlayWithState
    NetStart start feed
    factomParams
    Params
    util/config params
    dbStateManager/UpdateState EmitDirectoryBlockCommitEvent
    AddToProcessList EmitStateChangeEvent(m, eventmessages.EntityState_ACCEPTED)
    AddToHolding s.EventService.EmitRegistrationEvent(msg)
    DeleteFromHolding s.EventService.EmitStateChangeEvent(msg, eventmessages.EntityState_REJECTED)
    MoveStateToHeight EmitProcessListEventNewBlock, EmitProcessListEventNewMinute
    DoProcessing s.EventService.EmitNodeInfoMessageF(eventmessages.NodeMessageCode_STARTED, "Node %s startup complete", s.GetFactomNodeName())
    shutdown state.EventService.EmitNodeInfoMessageF(eventmessages.NodeMessageCode_SHUTDOWN,
        "Node %s is shutting down", state.GetFactomNodeName())
    state.EmitDirectoryBlockEventsFromHeight
    +EventService / GetEventService / EmitDirectoryBlockEventsFromHeight

?  NPN Peers, reworked to FromPeerToPeer in wax, deferred a lot of the logic to BMV?
ok engine/timer.go rework
ok state/MMR.go ReportMissing, Ask changes
ok dbStateCatchup factomsecond calculation
ok dbStateManager added containsServerWithChainID, a lot of Getters

ok state/instrumentation
    LeaderSyncAckDelay 
    LeaderSyncMsgDelay 
    LeaderSyncAckPairDelay 

ok state/processList.go
    GetKeysNewEntries keys init
    GetNewEntry nil check
    + CountFederatedServersAddedAndRemoved
    GetAckAt remove highestask check
    RemoveFromPL remove highestack check
    AddToProcessList leaderheight check

?  state/replay.go
    ok IsTSValidAndUpdateState check for internal replay
    validate check for internal replay
        commented out in wax https://github.com/FactomProject/factomd/blob/35d3a7ea30c0f936cbaf0ac85aece0dce3b9efb7/state/replay.go#L245-L250

ok state/safeMsgMap Cleanup refactor

ok state/state.go
    ok longTps (CalculateTransactionRate)
    ?  dataQueue -- not present in wax, handled by msg.Execute
    ok RequestTimeout from int to time.Duration
    ok ProcessTime

    ok GetPendingTransactions no fees for coinbase
    ok SetMessageFilterTimestamp refactor
    ok GotHeartbeat message filter, replay recenter commented out

ok state/stateConsensus.go
    ok - GetFilterTimeNano()
    ok Process priority queue change
    ok ProcessEOM s.EOMSyncEnd moved up

ok state/validation.go
    ok DoProcessing add p3 to p1 and p2

ok atomic.go WhereAmIString len check
ok util/misc -Trace

ok wsapi/ack.go handleAckByEntryHash tx time
ok wsapi/debugapi HandleDebugRequest, + write-configuration, wait-*, +errors
    ok HandleNetworkInfo +NodeName/Role
ok wsapi/instrumentation HandleV2APICallReplayDBFromHeight 
ok wsapi/server IDInjector
    ok HandleV2Diagnostics election bug

The main differences are:

WhoSoup commented 3 years ago

Incorporated the fixes from the sync bug hunt:

WhoSoup commented 3 years ago

Adding PR https://github.com/WhoSoup/factomd/pull/2, changes:

WhoSoup commented 3 years ago

https://github.com/WhoSoup/factomd/pull/3:

WhoSoup commented 3 years ago

I integrated the updated events module in develop into the merge, replacing the outdated wax version. It seems that the events module was restructured to include the same idea wax had, which unfortunately means there's a dual implementation right now. The functionality of wax depends on a few of these things, so for the moment we can't just remove the old events module.

The major change is:

It's currently working, though I haven't done any extensive testing on the module itself. Verified via https://github.com/WhoSoup/factom-eventprinter receiving a steady stream of events.

Examples of duplication: https://github.com/WhoSoup/factomd/blob/60facf958767b98b1624ad918fe116128af4beb3/state/stateConsensus.go#L87-L90

https://github.com/WhoSoup/factomd/blob/60facf958767b98b1624ad918fe116128af4beb3/state/validation.go#L27-L30

WhoSoup commented 3 years ago

Integrated the EntrySync rework (#1044) into this branch. This also addresses the issue #1058.

Major change is: