Skip to content

ExtractDestination is disabled to have vsolutions size > 1#429

Merged
ca333 merged 4 commits intoGLEECBTC:devfrom
dimxy:dev-komodo-vsolution-fix
May 15, 2021
Merged

ExtractDestination is disabled to have vsolutions size > 1#429
ca333 merged 4 commits intoGLEECBTC:devfrom
dimxy:dev-komodo-vsolution-fix

Conversation

@dimxy
Copy link
Copy Markdown
Collaborator

@dimxy dimxy commented May 12, 2021

disable vSolutions size > 1 after June2021 HF as it might break addressindex (this was mistakenly pulled from verus code)


bool komodo_Is2021JuneHFActive()
{
return GetLatestTimestamp(komodo_currentheight()) > nS5Timestamp;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

any specific reason why we use timestamp as opposed to block height check? As you know for Komodo blockchain it is komodo hardfork-height != hardfork-timestamp and difference can be significant

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I guess timestamp check only is used bcz this change is related to CC, and CC are active only in smartchains for now. But may be will be better to define komodo_Is2021JuneHFActive as:

bool komodo_Is2021JuneHFActive(uint32_t time)
{
    return ( (ASSETCHAINS_SYMBOL[0] == 0 && chainActive.Height() > nS5HardforkHeight) || (ASSETCHAINS_SYMBOL[0] != 0 && time > nS5Timestamp) );
}

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

this fix is supposed for cc chains (maybe it is better to rename the komodo_Is2021JuneHFActive() function to clarify it is timestamp based)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

this function was designed as self-contained, with no params, as it is called only one-time to fix one specific potential issue in standard.cpp (where we do not have neither chain time nor height)

Copy link
Copy Markdown

@DeckerSU DeckerSU May 12, 2021

Choose a reason for hiding this comment

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

i see. just offered a universal function, that can be used in other places too if needed. bcz currently we haven't any other function to determine is the S5 hardfork active or not.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

the code is not active on any chains other than ac_cc, so it's safe to do only the timestamp activation IMO

@dimxy dimxy changed the title ExtractDestination is disable to have vsolutions size > 1 ExtractDestination is disabled to have vsolutions size > 1 May 13, 2021
@dimxy
Copy link
Copy Markdown
Collaborator Author

dimxy commented May 13, 2021

I am having build problems on linux with this PR - do not merge!!
Some komodo utilities like komodo-tx, wallet-utility, komodo-test could not be built with this fix.
A linker undefined reference error occurs when linking them: libbitcoin_common_a lib (where standard.cpp resides) refers to symbols in the libbitcoin_server.a lib (which is not included into those utilities)

added the func's stubs to allow utils to build
@dimxy
Copy link
Copy Markdown
Collaborator Author

dimxy commented May 14, 2021

updated the PR, added a time-activation function stub in the utils to allow the utils to build

@DeckerSU DeckerSU mentioned this pull request May 15, 2021
@ca333 ca333 merged commit caba74c into GLEECBTC:dev May 15, 2021
DeckerSU added a commit to DeckerSU/KomodoOcean that referenced this pull request May 16, 2021
(c) GLEECBTC/komodo-daemon#429

TODO: consider possibility of remove any conditions based on timestamp
in standart.cpp after S5 hardfork, leaving only "after hardfork" branch.
Need to check all CC enabled chains sync from scratch to have clear is
removing conditions safe or not.
who-biz pushed a commit to who-biz/komodo that referenced this pull request Jun 14, 2022
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.

4 participants