Update Regtest Specific Params #676

tynes · 2019-01-25T19:11:48Z

This pull request updates the key prefixes and address prefixes to match bitcoind and btcd.
Addresses this issue: #579
See https://github.com/satoshilabs/slips/blob/master/slip-0173.md for a list of human readable prefixes

It build on #580 and reverts the change that sets the segwitstartTime to -1
https://github.com/bcoin-org/bcoin/pull/580/files#diff-44d3bcd9142fb799b54cb36cac72561eR804.

startTime is written and read from leveldb as a uint, as seen here:
https://github.com/bcoin-org/bcoin/blob/master/lib/blockchain/chaindb.js#L648
so setting it to -1 will cause an underflow and breaks some tests

codecov-io · 2019-01-28T23:38:45Z

Codecov Report

Merging #676 into master will increase coverage by 0.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #676      +/-   ##
==========================================
+ Coverage   53.38%   53.39%   +0.01%     
==========================================
  Files         104      104              
  Lines       27709    27711       +2     
  Branches     4746     4747       +1     
==========================================
+ Hits        14792    14797       +5     
+ Misses      12917    12914       -3

Impacted Files	Coverage Δ
lib/protocol/networks.js	`100% <ø> (ø)`	⬆️
lib/blockchain/chain.js	`60.62% <100%> (+0.07%)`	⬆️
lib/blockchain/chaindb.js	`56.88% <100%> (+0.09%)`	⬆️
lib/mining/template.js	`87.32% <0%> (-0.85%)`	⬇️
lib/node/rpc.js	`23.58% <0%> (+0.19%)`	⬆️
lib/coins/compress.js	`56.32% <0%> (+1.26%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7f736db...b016cb0. Read the comment docs.

pinheadmz · 2019-01-29T03:36:23Z

Confirmed these match bitcoin core regtest params: https://github.com/bitcoin/bitcoin/blob/master/src/chainparams.cpp#L342-L348
...even though they're base58 prefixes are encoded in... decimal?!

Also compiled Bitcoin Core from source, ran it in regtest and networked it with bcoin in regtest running locally. Ran my txmess script to generate 100s of blocks and txs, and the nodes synced fine. Randomly picked a block and printed all the txs | grep address from both nodes JUST to make extra double hella sure Core generates the same addresses as bcoin in regtest after this PR

tuxcanfly

OK

pinheadmz · 2019-01-31T18:15:33Z

Also just confirming Bitcoin Core regtest SegWit is "always active" so no deployment time:

https://github.com/bitcoin/bitcoin/blob/4b6673d3826128a9f58fd5417aaf4c2f4169c6aa/src/consensus/params.h#L39-L43

https://github.com/bitcoin/bitcoin/blob/4b6673d3826128a9f58fd5417aaf4c2f4169c6aa/src/chainparams.cpp#L298

braydonf · 2019-01-31T18:17:46Z

So I'm still getting an issue with connecting a Bitcoin Core regtest node and Bcoin regtest node here because of the activation time.

[debug] (peer) Rejecting block 5803d64a9bd67fc914c7b05db3beae3e2308b819578629beaeea12764bd890c1 (127.0.0.1:18444): code=invalid reason=unexpected-witness.

The change here 53a09b0 with the activation was necessary for that.

pinheadmz · 2019-01-31T18:30:23Z

@braydonf What's your process? I networked bcoin and Bitcoin Core in regtest, and in bcoin:

bcoin-cli --http-port=8888 rpc generatetoaddress 1 bcrt1qa2v6v49gftjekdkulwz8zj7vj6ln26j0te9ldw

Bitcoin Core shows the block in the chain just fine (even though coinbase address was segwit)

./bitcoin-cli -regtest getblock `./bitcoin-cli -regtest getblockhash 1`
{
  "hash": "43c169241d289eda42cc1388a1576b089111c5a3e1a821c97ca9400f208608fe",
  "confirmations": 1,
  "strippedsize": 193,
  "size": 193,
  "weight": 772,
  "height": 1,
  "version": 536870912,
  "versionHex": "20000000",
  "merkleroot": "d866f842b06ac48cf84d794fb5ef2dc4d216e286f46929c6c5e3cdf74d9a7a22",
  "tx": [
    "d866f842b06ac48cf84d794fb5ef2dc4d216e286f46929c6c5e3cdf74d9a7a22"
  ],
  "time": 1548959281,
  "mediantime": 1548959281,
  "nonce": 1,
  "bits": "207fffff",
  "difficulty": 4.656542373906925e-10,
  "chainwork": "0000000000000000000000000000000000000000000000000000000000000004",
  "nTx": 1,
  "previousblockhash": "0f9188f13cb7b2c71f2a335e3a4fc328bf5beb436012afca590b1a11466e2206"
}

pinheadmz · 2019-01-31T18:32:22Z

Oh weird - ok, however bcoin rejected a segwit block from Core:

[warning] (chain) Tried to connect invalid block: 3665d2bd2df0f13b3a2fbced0f384c85328cddf5883986b30da551cee195e459 (2).
[debug] (peer) Rejecting block 3665d2bd2df0f13b3a2fbced0f384c85328cddf5883986b30da551cee195e459 (127.0.0.1:62908): code=invalid reason=unexpected-witness.
[debug] (peer) Sending reject packet to peer (127.0.0.1:62908).
[debug] (peer) Ban threshold exceeded (127.0.0.1:62908).
[debug] (net) Banning peer (127.0.0.1:62908).
[warning] (net) Error: Verification failure: unexpected-witness (code=invalid score=100 hash=3665d2bd2df0f13b3a2fbced0f384c85328cddf5883986b30da551cee195e459)

braydonf · 2019-01-31T18:33:42Z

Here are my steps for testing:

Start and connect Bitcoin Core and Bcoin nodes:

./bitcoin/src/bitcoind -regtest -printtoconsole
./bin/bcoin --network=regtest --nodes=127.0.0.1:18444

Make sure that the regtest chain data is clear for each for a fresh chain and that the nodes connect. Your port configurations may vary.

Getting blockchain information from Bitcoin Core:

./bitcoin/src/bitcoin-cli -regtest getblockchaininfo

Getting blockchain information from Bcoin:

./bitcoin/src/bitcoin-cli -regtest -rpcport=48332 getblockchaininfo

Generating blocks:

./bitcoin/src/bitcoin-cli -regtest getnewaddress
./bitcoin/src/bitcoin-cli -regtest generatetoaddress 1 <address>

And then verifying that both chains have the block using the above commands.

pinheadmz · 2019-01-31T18:34:32Z

Actually I can confirm with rpc getblockchaininfo

Bitcoin Core:

    "segwit": {
      "status": "active",
      "startTime": -1,
      "timeout": 9223372036854775807,
      "since": 0
    }

bcoin:

    "segwit": {
      "status": "defined",
      "bit": 1,
      "startTime": 0,
      "timeout": 4294967295
    },

oops wont sync w core

pinheadmz · 2019-01-31T18:40:40Z

Gonna revert b10aea8 and test more

braydonf · 2019-01-31T21:50:20Z

Tested, and is syncing and the deployments cache will be refreshed.

braydonf

LGTM after rebase on master.

The deployments are written to and read from a database and the start time is a U32. Changing the value to -1 will cause the value to be read incorrectly and cause the tests to fail.

tynes · 2019-01-31T22:14:22Z

Rebased

aquaflamingo · 2019-02-05T04:08:24Z

possible to get this merged?

Update Regtest Specific Params

braydonf · 2019-02-06T16:41:01Z

Merged at 39684df

tynes requested review from tuxcanfly, boymanjor, nodech and pinheadmz January 25, 2019 19:11

pinheadmz mentioned this pull request Jan 27, 2019

Why are regtest addresses prefixed with 'R' instead of 'm' which is common in other btc node clients? #680

Closed

tynes force-pushed the tynes/update-regtest branch from 8c9a188 to b10aea8 Compare January 28, 2019 23:38

tuxcanfly approved these changes Jan 31, 2019

View reviewed changes

pinheadmz previously approved these changes Jan 31, 2019

View reviewed changes

tynes force-pushed the tynes/update-regtest branch from b10aea8 to 177403f Compare January 31, 2019 21:35

braydonf approved these changes Jan 31, 2019

View reviewed changes

tuxcanfly and others added 5 commits January 31, 2019 14:06

networks: update regtest params

187f015

networks: always activate segwit for regtest

d9529b2

networks: restore regtest ports

36f8c1a

bugfix: regtest segwit start time 0

b30881d

The deployments are written to and read from a database and the start time is a U32. Changing the value to -1 will cause the value to be read incorrectly and cause the tests to fail.

blockchain: always enable segwit for regtest

b016cb0

tynes force-pushed the tynes/update-regtest branch from 177403f to b016cb0 Compare January 31, 2019 22:08

braydonf mentioned this pull request Feb 1, 2019

networks: update regtest params #580

Closed

nodech approved these changes Feb 5, 2019

View reviewed changes

braydonf added a commit to braydonf/bcoin that referenced this pull request Feb 5, 2019

Merge pull request bcoin-org#676 from tynes/update-regtest

39684df

Update Regtest Specific Params

braydonf closed this Feb 6, 2019

braydonf mentioned this pull request Feb 6, 2019

network: regtest params #579

Closed

tynes deleted the tynes/update-regtest branch February 6, 2019 21:45

pinheadmz mentioned this pull request Feb 6, 2019

Allow prematureWitness flag #528

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update Regtest Specific Params #676

Update Regtest Specific Params #676

tynes commented Jan 25, 2019

codecov-io commented Jan 28, 2019 •

edited

Loading

pinheadmz commented Jan 29, 2019

tuxcanfly left a comment

pinheadmz commented Jan 31, 2019

braydonf commented Jan 31, 2019

pinheadmz commented Jan 31, 2019

pinheadmz commented Jan 31, 2019

braydonf commented Jan 31, 2019

pinheadmz commented Jan 31, 2019

pinheadmz commented Jan 31, 2019

braydonf commented Jan 31, 2019 •

edited

Loading

braydonf left a comment

tynes commented Jan 31, 2019

aquaflamingo commented Feb 5, 2019

braydonf commented Feb 6, 2019

Update Regtest Specific Params #676

Update Regtest Specific Params #676

Conversation

tynes commented Jan 25, 2019

codecov-io commented Jan 28, 2019 • edited Loading

Codecov Report

pinheadmz commented Jan 29, 2019

tuxcanfly left a comment

Choose a reason for hiding this comment

pinheadmz commented Jan 31, 2019

braydonf commented Jan 31, 2019

pinheadmz commented Jan 31, 2019

pinheadmz commented Jan 31, 2019

braydonf commented Jan 31, 2019

pinheadmz commented Jan 31, 2019

Bitcoin Core:

bcoin:

pinheadmz commented Jan 31, 2019

braydonf commented Jan 31, 2019 • edited Loading

braydonf left a comment

Choose a reason for hiding this comment

tynes commented Jan 31, 2019

aquaflamingo commented Feb 5, 2019

braydonf commented Feb 6, 2019

codecov-io commented Jan 28, 2019 •

edited

Loading

braydonf commented Jan 31, 2019 •

edited

Loading