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

DAA is not compatible with reference implementation code. #11

Closed
fpelliccioni opened this issue Feb 28, 2020 · 2 comments · Fixed by #12
Closed

DAA is not compatible with reference implementation code. #11

fpelliccioni opened this issue Feb 28, 2020 · 2 comments · Fixed by #12
Assignees
Labels
bug Something isn't working

Comments

@fpelliccioni
Copy link

fpelliccioni commented Feb 28, 2020

Bitcoin Verde DAA implementation is not compatible with of the other implementations.
This is not the fault of Bitcoin Verde, but the fault of how badly specified DAA is and how poorly implemented by ABC.
Verde and Knuth are the only 2 node implementations that have written DAA from scratch, the rest of the nodes have only copied or translated the Bitcoin ABC code, so they don't have this issue.

Here is a reference in this regard:
https://read.cash/@Fernando/on-daa-implementation-algorithms-and-specifications-b739e631

Given the following code, Bitcoin Verde prints "1" and Bitcoin ABC prints "2", so the consensus rules differ.
I would have liked to try this on testnet to probe it in a better way, generating a similar 3-blocks pattern, but I haven't had enough HP to do it.

package app;

import java.math.BigInteger;
import java.util.Arrays;
import java.util.Comparator;

final class BlockHeader {
    public Long Height;
    public Long Timestamp;
}

final class DifficultyCalculator {
    public BlockHeader[]  _calculateNewBitcoinCashTarget(final BlockHeader[] firstBlockHeaders, final BlockHeader[] lastBlockHeaders) {
        final Comparator<BlockHeader> sortBlockHeaderByTimestampDescending = new Comparator<BlockHeader>() {
            @Override
            public int compare(final BlockHeader blockHeader0, final BlockHeader blockHeader1) {
                return blockHeader1.Timestamp.compareTo(blockHeader0.Timestamp);
            }
        };

        Arrays.sort(lastBlockHeaders, sortBlockHeaderByTimestampDescending);
        Arrays.sort(firstBlockHeaders, sortBlockHeaderByTimestampDescending);

        final BlockHeader firstBlockHeader = firstBlockHeaders[1];
        final BlockHeader lastBlockHeader = lastBlockHeaders[1];

        final BlockHeader[] res = {firstBlockHeader, lastBlockHeader};
        return res;
    }
}

public class App {
    public static void main(final String[] args) throws Exception {

        final BlockHeader[] firstBlockHeaders = new BlockHeader[3]; // The oldest BlockHeaders...

        firstBlockHeaders[0] = new BlockHeader();
        firstBlockHeaders[0].Timestamp = (long) 1558731500;
        firstBlockHeaders[0].Height = (long) 3;
        firstBlockHeaders[1] = new BlockHeader();
        firstBlockHeaders[1].Timestamp = (long) 1558731501;
        firstBlockHeaders[1].Height = (long) 2;
        firstBlockHeaders[2] = new BlockHeader();
        firstBlockHeaders[2].Timestamp = (long) 1558731501;
        firstBlockHeaders[2].Height = (long) 1;

        final BlockHeader[] lastBlockHeaders = new BlockHeader[3]; // The newest BlockHeaders...
        lastBlockHeaders[0] = new BlockHeader();
        lastBlockHeaders[0].Timestamp = (long) 1558731500;
        lastBlockHeaders[0].Height = (long) 3;
        lastBlockHeaders[1] = new BlockHeader();
        lastBlockHeaders[1].Timestamp = (long) 1558731500;
        lastBlockHeaders[1].Height = (long) 2;
        lastBlockHeaders[2] = new BlockHeader();
        lastBlockHeaders[2].Timestamp = (long) 1558730000;
        lastBlockHeaders[2].Height = (long) 1;

        DifficultyCalculator c = new DifficultyCalculator();
        final BlockHeader[] selected = c._calculateNewBitcoinCashTarget(firstBlockHeaders, lastBlockHeaders);

        System.out.println(selected[0].Height);
    }
}

@fpelliccioni
Copy link
Author

This also affects the efforts to create a specification for BCH. We would have to adjust how DAA is specified.

@joshmg joshmg self-assigned this Feb 29, 2020
@joshmg joshmg added the bug Something isn't working label Feb 29, 2020
@joshmg
Copy link
Member

joshmg commented Feb 29, 2020

Thanks for reporting this, Fernando. I've verified this behavior in the specification as well in ABC's code.
I've created a replicating test case and a resolution to this issue. PR #12 will resolve this issue once tests are executed and a full chain validation is successful on our test node.

@joshmg joshmg linked a pull request Feb 29, 2020 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants