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

[WIP, 0.17] Add pegin validation #458

Open
wants to merge 26 commits into
base: elements-0.17
from

Conversation

Projects
None yet
3 participants
@stevenroose
Member

stevenroose commented Nov 7, 2018

No description provided.

@stevenroose stevenroose changed the base branch from elements-0.14.1 to elements-0.17 Nov 7, 2018

@stevenroose stevenroose changed the title from [WIP; 0.17] Add pegin validation to [WIP, 0.17] Add pegin validation Nov 7, 2018

@stevenroose stevenroose referenced this pull request Nov 7, 2018

Open

Update elements to Bitcoin Core version 0.17.0 #398

4 of 22 tasks complete
gArgs.AddArg("-secretprefix", strprintf("The byte prefix, in decimal, of the chain's base58 secret key encoding. (default: %d)", 239), false, OptionsCategory::ELEMENTS);
gArgs.AddArg("-extpubkeyprefix", strprintf("The 4-byte prefix, in hex, of the chain's base58 extended public key encoding. (default: %s)", "043587CF"), false, OptionsCategory::ELEMENTS);
gArgs.AddArg("-extprvkeyprefix", strprintf("The 4-byte prefix, in hex, of the chain's base58 extended private key encoding. (default: %s)", "04358394"), false, OptionsCategory::ELEMENTS);
gArgs.AddArg("-bech32_hrp", strprintf("The human-readable part of the chain's bech32 encoding. (default: %s)", "bc"), false, OptionsCategory::ELEMENTS);

This comment has been minimized.

@luke-jr

luke-jr Nov 8, 2018

Contributor

Why is this Bitcoin's default, while the base58 ones are not?

This comment has been minimized.

@instagibbs

instagibbs Nov 8, 2018

Collaborator

because we haven't picked one for elements yet!

In general I expect us to take the network name specifically in the hrp, like liquidv, but for elements it can be more generic.

ele?

This comment has been minimized.

@stevenroose

stevenroose Nov 9, 2018

Member

I think the base58 defaults are regtest's if I'm not mistaken. So perhaps this should be "bcrt"?

This comment has been minimized.

@stevenroose

stevenroose Nov 9, 2018

Member

Ah no this is not for parent. Yeah this is because we don't have one for elements yet.

gArgs.AddArg("-extprvkeyprefix", strprintf("The 4-byte prefix, in hex, of the chain's base58 extended private key encoding. (default: %s)", "04358394"), false, OptionsCategory::ELEMENTS);
gArgs.AddArg("-bech32_hrp", strprintf("The human-readable part of the chain's bech32 encoding. (default: %s)", "bc"), false, OptionsCategory::ELEMENTS);
gArgs.AddArg("-mainchainrpchost=<host>", "The address which the daemon will try to connect to the trusted bitcoind to validate peg-ins, if enabled. (default: cookie auth)", false, OptionsCategory::ELEMENTS);
gArgs.AddArg("-mainchainrpcport=<n>", "The port which the daemon will try to connect to the trusted bitcoind to validate peg-ins, if enabled. (default: cookie auth)", false, OptionsCategory::ELEMENTS);

This comment has been minimized.

@luke-jr

luke-jr Nov 8, 2018

Contributor

host/port are used even for cookie auth.

This comment has been minimized.

@stevenroose

stevenroose Nov 9, 2018

Member

Thanks. That was copy paste, but will take it away.

gArgs.AddArg("-mainchainrpcpassword=<pwd>", "The rpc password which the daemon will use to connect to the trusted bitcoind to validate peg-ins, if enabled. (default: cookie auth)", false, OptionsCategory::ELEMENTS);
gArgs.AddArg("-mainchainrpccookiefile=<file>", "The bitcoind cookie auth path which the daemon will use to connect to the trusted bitcoind to validate peg-ins. (default: `<datadir>/regtest/.cookie`)", false, OptionsCategory::ELEMENTS);
gArgs.AddArg("-mainchainrpctimeout=<n>", strprintf("Timeout in seconds during mainchain RPC requests, or 0 for no timeout. (default: %d)", DEFAULT_HTTP_CLIENT_TIMEOUT), false, OptionsCategory::ELEMENTS);
gArgs.AddArg("-validatepegin", strprintf("Validate peg-in claims. An RPC connection will be attempted to the trusted bitcoind using the `mainchain*` settings below. All functionaries must run this enabled. (default: %u)", DEFAULT_VALIDATE_PEGIN), false, OptionsCategory::ELEMENTS);

This comment has been minimized.

@luke-jr

luke-jr Nov 8, 2018

Contributor

"below" is wrong

This comment has been minimized.

@stevenroose
if (!anyonecanspend_aremine) {
assert("Anyonecanspendismine was marked as false, but they are in the genesis block"
&& initialFreeCoins == 0);

This comment has been minimized.

@luke-jr

luke-jr Nov 8, 2018

Contributor
chainparams.cpp:506:24: error: ‘initialFreeCoins’ was not declared in this scope

This comment has been minimized.

@stevenroose

stevenroose Nov 9, 2018

Member

Yeah, it's a WIP PR, so there's plenty of loose ends still. I'm going through the process of recursively adding things that I missed :/

// index field. The IssuanceMode enum values are chosen to
// make this as simple as a bitwise-OR.
outpoint.hash = prevout.hash;
outpoint.n = prevout.n & COutPoint::OUTPOINT_INDEX_MASK;

This comment has been minimized.

@luke-jr

luke-jr Nov 15, 2018

Contributor

This is your genesis block problem. A coinbase's outpoint has n==0xffffffff, which is being masked off here.

This comment has been minimized.

@stevenroose

stevenroose Nov 16, 2018

Member

YES! Thanks! I was looking at this line, but somehow assumed the coinbase outpoint was 0..

READWRITE(outpoint);
if (ser_action.ForRead()) {
//if (outpoint.n == (uint32_t) -1) {

This comment has been minimized.

@instagibbs

instagibbs Nov 16, 2018

Collaborator

this needs to be active to not have coinbase outpoints be invalid as @luke-jr said

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment