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.

Sign up for GitHub

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

Jump to bottom

Additional review of prop 29 and migration testing #559

Merged
merged 5 commits into from
Jan 25, 2021
Merged

Additional review of prop 29 and migration testing #559

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
23a4e59
Partial resolution on issues with Ethereum signature verification
zmanian Jan 22, 2021
99b606c
More signature verification fixes
zmanian Jan 22, 2021
5957a5a
Switch to strings.ReplaceAll
zmanian Jan 22, 2021
1e1b8dd
All signatures verify now
zmanian Jan 22, 2021
cb0e824
cleanup
fedekunze Jan 22, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
185 changes: 110 additions & 75 deletions app/prop29.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ import (
"bytes"
"encoding/base64"
"encoding/json"
"fmt"
"log"
"strings"

"github.com/btcsuite/btcd/btcec"
"github.com/btcsuite/btcd/chaincfg"
Expand All @@ -20,7 +22,9 @@ import (
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
distr "github.com/cosmos/cosmos-sdk/x/distribution/types"

"github.com/ethereum/go-ethereum/accounts"
"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/common/hexutil"
"github.com/ethereum/go-ethereum/crypto"
)
Expand Down Expand Up @@ -59,6 +63,7 @@ func (r *recoveryMessages) GetRemainingBalances() (balances []banktypes.Balance)
return
}

// GetRemainingAccounts returns the recovery destination addresses with positive balance
func (r *recoveryMessages) GetRemainingAccounts() (addresses []sdk.Address) {
zeroBalance := sdk.NewInt64Coin("uatom", 0)
for _, m := range *r {
Expand All @@ -67,7 +72,7 @@ func (r *recoveryMessages) GetRemainingAccounts() (addresses []sdk.Address) {
}

}
return
return addresses
}

// FundRecoveryMessage were signed messages provided by fundraiser particpants who could not access their ATOM to facilate this process for recovering access to their funds.
Expand All @@ -80,7 +85,8 @@ type FundRecoveryMessage struct {
}

func (f *FundRecoveryMessage) unMarshalSignedJSON() (signedJSON, error) {
return unmarshalSignedJSON([]byte(f.signedMessage))
newLineStripped := strings.ReplaceAll(f.signedMessage, `\n`, "")
return unmarshalSignedJSON([]byte(strings.ReplaceAll(newLineStripped, "\\", "")))
}

// nolint: unparam
Expand All @@ -89,7 +95,12 @@ func (f *FundRecoveryMessage) verifyBitcoinSignature(addrIdx int) (signedJSON, e
if err != nil {
return signedJSON{}, err
}
verifyBitcoinSignature(f.signature, f.signedMessage, msgJSON.ContributingAddresses[addrIdx].Address)

err = verifyBitcoinSignature(f.signature, f.signedMessage, msgJSON.ContributingAddresses[addrIdx].Address)
if err != nil {
return signedJSON{}, err
}

return msgJSON, nil
}

Expand All @@ -100,7 +111,11 @@ func (f *FundRecoveryMessage) verifyEthereumSignature(addrIdx int) (signedJSON,
return signedJSON{}, err
}

verifyEthereumSignature(f.signature, f.signedMessage, msgJSON.ContributingAddresses[addrIdx].Address)
err = verifyEthereumSignature(f.signature, f.signedMessage, msgJSON.ContributingAddresses[addrIdx].Address)
if err != nil {
return signedJSON{}, err
}

return msgJSON, nil
}

Expand Down Expand Up @@ -186,8 +201,8 @@ func ethDonor1() FundRecoveryMessage {
destAddress, _ := sdk.AccAddressFromBech32("cosmos17cg9xxpjnammafyqwfryr3hn32h6vjmh9x0y6j") // nolint: errcheck

return FundRecoveryMessage{
`{ \"Message\":\"I remember After I successfully sent my 16 Eth to the Cosmos fundraising address 3 years ago,I wrote down those seed words and had a picture.But when we could claim,I found my seed phase does not work,I've tried every means I could,but all failed.Honestly,I still don’t know the problem,maybe I wrote wrong letters,maybe something wrong with that webpage,or maybe I mixed something,only God knows.\", \"Txid\":\"https://etherscan.io/tx/0x42d0f860e1cd484f51647f34479843008b69d8f1158c94ad44ae30df33fdc080\", \"Contribution\":\"16 ETH\", \"Contributing Addresses\":[ { \"address\":\"0x53ad4398f76a453a2d4dac4470f0b81cd1d72715\" } ], \"Genesis Cosmos Address\":\"cosmos1m06fehere8q09sqcj08dcc0xyj3fjlzc2x24y4\", \"Recovery Cosmos Address\":\"cosmos17cg9xxpjnammafyqwfryr3hn32h6vjmh9x0y6j\" }`,
"ceed630f7e8d102b125a22d9ec06ced12a016f376b408b9832a63a9b4b5f352b4b67c94e9cfa35e4f2676f0f92643f8b56caf82f6c6eabd765b787d3f6af77fb1b",
`{ "Message":"I remember After I successfully sent my 16 Eth to the Cosmos fundraising address 3 years ago,I wrote down those seed words and had a picture.But when we could claim,I found my seed phase does not work,I've tried every means I could,but all failed.Honestly,I still don’t know the problem,maybe I wrote wrong letters,maybe something wrong with that webpage,or maybe I mixed something,only God knows.", "Txid":"https://etherscan.io/tx/0x42d0f860e1cd484f51647f34479843008b69d8f1158c94ad44ae30df33fdc080", "Contribution":"16 ETH", "Contributing Addresses":[ { "address":"0x53ad4398f76a453a2d4dac4470f0b81cd1d72715" } ], "Genesis Cosmos Address":"cosmos1m06fehere8q09sqcj08dcc0xyj3fjlzc2x24y4", "Recovery Cosmos Address":"cosmos17cg9xxpjnammafyqwfryr3hn32h6vjmh9x0y6j" }`,
"0xceed630f7e8d102b125a22d9ec06ced12a016f376b408b9832a63a9b4b5f352b4b67c94e9cfa35e4f2676f0f92643f8b56caf82f6c6eabd765b787d3f6af77fb1b",
sourceAddress,
destAddress,
sdk.NewInt64Coin("uatom", 6512400000)}
Expand All @@ -198,8 +213,8 @@ func ethDonor2() FundRecoveryMessage {
destAddress, _ := sdk.AccAddressFromBech32("cosmos1teux7wdnnq03r7r277yu762mq3cket5mg4xd3e") // nolint: errcheck

return FundRecoveryMessage{
`{\n \"Message\":\"In April 2017, I participated in 24 ETH at Cosmos ico. In about two years when Cosmos was listed on exchange, the cell phone that had taken the seed phrase picture was destroyed. I tried to go to a data recovery company, but I couldn't recover the picture, so there was no way to get ATOM. However, I still have the private key and keystore file of the Ethereum wallet that participated in Cosmos ico.\",\n \"Txid\":\"https://etherscan.io/tx/0xe2bb8c832c237b9ed898d4616649347e84931d56b8942cb409cafd6b01e1913d\",\n \"Contribution\":\"24 ETH\",\n \"Contributing Addresses\":[\n {\n \"address\":\"0xff3fa81a59f31bd563d2554401438a1678d43593\"\n }\n ],\n \"Genesis Cosmos Address\":\"cosmos1gzuqry88awndjjsa5exzx4gwnmktcpdrxgdcf6\",\n \"Recovery Cosmos Address\":\"cosmos1teux7wdnnq03r7r277yu762mq3cket5mg4xd3e\"\n}\n`,
"38de4018152de5f42d24b1150c04f5010dc3edce9a3436a57d318beae5e6955228a5e2c1255591c0324e4e9f1bbd13806e51bbdb11259c9e2aeddbdbc91bc11a1b",
"{\n \"Message\":\"In April 2017, I participated in 24 ETH at Cosmos ico. In about two years when Cosmos was listed on exchange, the cell phone that had taken the seed phrase picture was destroyed. I tried to go to a data recovery company, but I couldn't recover the picture, so there was no way to get ATOM. However, I still have the private key and keystore file of the Ethereum wallet that participated in Cosmos ico.\",\n \"Txid\":\"https://etherscan.io/tx/0xe2bb8c832c237b9ed898d4616649347e84931d56b8942cb409cafd6b01e1913d\",\n \"Contribution\":\"24 ETH\",\n \"Contributing Addresses\":[\n {\n \"address\":\"0xff3fa81a59f31bd563d2554401438a1678d43593\"\n }\n ],\n \"Genesis Cosmos Address\":\"cosmos1gzuqry88awndjjsa5exzx4gwnmktcpdrxgdcf6\",\n \"Recovery Cosmos Address\":\"cosmos1teux7wdnnq03r7r277yu762mq3cket5mg4xd3e\"\n}\n",
"0x38de4018152de5f42d24b1150c04f5010dc3edce9a3436a57d318beae5e6955228a5e2c1255591c0324e4e9f1bbd13806e51bbdb11259c9e2aeddbdbc91bc11a1b",
sourceAddress,
destAddress,
sdk.NewInt64Coin("uatom", 9769500000)}
Expand All @@ -210,8 +225,8 @@ func ethDonor3() FundRecoveryMessage {
destAddress, _ := sdk.AccAddressFromBech32("cosmos18qjynuyrfja9qugzs4zjcs6dh0qyprqa2vwktp") // nolint: errcheck

return FundRecoveryMessage{
`{ \"Message\":\"One of the early Ethereum alternatives is Cosmos. This is one of the reasons ICONOMI that is a collective of 1500 small donors , donated to Cosmos fundraiser about 2,222 ETH similarly to numerous ICO where we participated. Due to the possible bug in the code, we did not get the seed phrases. During the subscription process using the Brave browser, following all the steps the seed phrases simply did not show-up. Iconomi will be grateful to the community for the recovery of the atoms.\", \"Txid\":\"https://etherscan.io/tx/0x0c85c7cc2b66840357c3f293ae2010f0c79f2cc9f4b1220028afe780fdfdb426\", \"Contribution\":\"2222 ETH\", \"Contributing Addresses\":[ { \"address\":\"0xb4dc54df11d2dcecd046e5c7318fb241a73ee370\" } ], \"Genesis Cosmos Address\":\"cosmos1r3xvguuhwvlk34esxclvrh3g7ycmcqqc2kcn9v\", \"Recovery Cosmos Address\":\"cosmos18qjynuyrfja9qugzs4zjcs6dh0qyprqa2vwktp\" }`,
"4c29f9d74a070a8c475553597a1bd461137af0ba9120c183a1cfe3dc8c729f367dcf76ed9a384eea18c920b7ba7613ddb5632da9642fdb42cc183ff5ea74614e1b",
"{ \"Message\":\"One of the early Ethereum alternatives is Cosmos. This is one of the reasons ICONOMI that is a collective of 1500 small donors , donated to Cosmos fundraiser about 2,222 ETH similarly to numerous ICO where we participated. Due to the possible bug in the code, we did not get the seed phrases. During the subscription process using the Brave browser, following all the steps the seed phrases simply did not show-up. Iconomi will be grateful to the community for the recovery of the atoms.\", \"Txid\":\"https://etherscan.io/tx/0x0c85c7cc2b66840357c3f293ae2010f0c79f2cc9f4b1220028afe780fdfdb426\", \"Contribution\":\"2222 ETH\", \"Contributing Addresses\":[ { \"address\":\"0xb4dc54df11d2dcecd046e5c7318fb241a73ee370\" } ], \"Genesis Cosmos Address\":\"cosmos1r3xvguuhwvlk34esxclvrh3g7ycmcqqc2kcn9v\", \"Recovery Cosmos Address\":\"cosmos18qjynuyrfja9qugzs4zjcs6dh0qyprqa2vwktp\" }",
"0x4c29f9d74a070a8c475553597a1bd461137af0ba9120c183a1cfe3dc8c729f367dcf76ed9a384eea18c920b7ba7613ddb5632da9642fdb42cc183ff5ea74614e1b",
sourceAddress,
destAddress,
sdk.NewInt64Coin("uatom", 904509000000)}
Expand All @@ -228,19 +243,19 @@ func validateFundRecovery() recoveryMessages {
//Bitcoin Verification
_, err := bDonor1.verifyBitcoinSignature(0)
if err != nil {
log.Fatal(err)
log.Fatalf("BTC1 %e", err)
}
_, err = bDonor2.verifyBitcoinSignature(0)
if err != nil {
log.Fatal(err)
log.Fatalf("BTC2 %e", err)
}
_, err = bDonor3.verifyBitcoinSignature(1)
if err != nil {
log.Fatal(err)
log.Fatalf("BTC3 %e", err)
}
_, err = bDonor4.verifyBitcoinSignature(0)
if err != nil {
log.Fatal(err)
log.Fatalf("BTC4 %e", err)
}

//Ethereum Donors
Expand All @@ -251,82 +266,96 @@ func validateFundRecovery() recoveryMessages {
//Ethereum Verification
_, err = eDonor1.verifyEthereumSignature(0)
if err != nil {
log.Fatal(err)
log.Fatalf("Eth1 %e", err)
}

_, err = eDonor2.verifyEthereumSignature(0)
if err != nil {
log.Fatal(err)
log.Fatalf("Eth2 %e", err)
}

_, err = eDonor3.verifyEthereumSignature(0)
if err != nil {
log.Fatal(err)
log.Fatalf("Eth3 %e", err)
}

return []FundRecoveryMessage{bDonor1, bDonor2, bDonor3, bDonor4, eDonor1, eDonor2, eDonor3}

}

// Eth_sign verifier for MEW signatures.

func verifyEthereumSignature(sig, msg, addr string) {
sigBytes, err := hexutil.Decode(sig)
// EcRecover returns the address for the account that was used to create the signature.
// Note, this function is compatible with eth_sign and personal_sign. As such it recovers
// the address of:
// hash = keccak256("\x19Ethereum Signed Message:\n"${message length}${message})
// addr = ecrecover(hash, signature)
//
// Note, the signature must conform to the secp256k1 curve R, S and V values, where
// the V value must be 27 or 28 for legacy reasons.
//
// https://github.com/ethereum/go-ethereum/wiki/Management-APIs#personal_ecRecover
func ecRecover(sig, msg hexutil.Bytes) (common.Address, error) {
if len(sig) != crypto.SignatureLength {
return common.Address{}, fmt.Errorf("signature must be %d bytes long", crypto.SignatureLength)
}
if sig[crypto.RecoveryIDOffset] != 27 && sig[crypto.RecoveryIDOffset] != 28 {
return common.Address{}, fmt.Errorf("invalid Ethereum signature (V is not 27 or 28)")
}
sig[crypto.RecoveryIDOffset] -= 27 // Transform yellow paper V from 27/28 to 0/1

rpk, err := crypto.SigToPub(accounts.TextHash(msg), sig)
if err != nil {
log.Fatal(err)
return common.Address{}, err
}
return crypto.PubkeyToAddress(*rpk), nil
}

addrBytes, err := hexutil.Decode(addr)
if err != nil {
log.Fatal(err)
}

hash := accounts.TextHash([]byte(msg))

sigPublicKey, err := crypto.Ecrecover(hash, sigBytes)
// Eth_sign verifier for MEW signatures.
func verifyEthereumSignature(sig, msg, addr string) error {
var sigBz hexutil.Bytes

if err != nil {
log.Fatal(err)
if err := sigBz.UnmarshalText([]byte(sig)); err != nil {
return fmt.Errorf("eth sig unmarshal error: %w", err)
}

pubkey, err := crypto.DecompressPubkey(sigPublicKey)
addrRecovered, err := ecRecover(sigBz, []byte(msg))
if err != nil {
log.Fatal(err)
return fmt.Errorf("ecRecover error: %w", err)
}

addrRecovered := crypto.PubkeyToAddress(*pubkey)
address := common.HexToAddress(addr)

if !bytes.Equal(addrRecovered.Bytes(), addrBytes) {
log.Fatal("invalid signature")
if !bytes.Equal(addrRecovered.Bytes(), address.Bytes()) {
log.Fatalf("invalid signature, given address %s, recovered address %s", address.String(), addrRecovered.String())
}

return nil
}

const messageSignatureHeader = "Bitcoin Signed Message:\n"

// Modified Bitcoin signature key recovery and address verification script that verifies
// signed messages against simple pay to pub key hash addresses.
// Should panic on failure.
func verifyBitcoinSignature(sig, msg, addr string) {

func verifyBitcoinSignature(sig, msg, addr string) error {
var buf bytes.Buffer
err := wire.WriteVarString(&buf, 0, messageSignatureHeader)
if err != nil {
log.Fatal(err)
return fmt.Errorf("msg signature header serialization failed: %w", err)
}

err = wire.WriteVarString(&buf, 0, msg)
if err != nil {
log.Fatal(err)
if err := wire.WriteVarString(&buf, 0, msg); err != nil {
return fmt.Errorf("msg serialization failed: %w", err)
}

expectedMessageHash := chainhash.DoubleHashB(buf.Bytes())

sigBytes, err := base64.StdEncoding.DecodeString(sig)
if err != nil {
log.Fatal(err)
return fmt.Errorf("base64 signature decoding failed: %w", err)
}

pk, wasCompressed, err := btcec.RecoverCompact(btcec.S256(), sigBytes, expectedMessageHash)
if err != nil {
log.Fatal(err)
return fmt.Errorf("signature verification failed: %w", err)
}

var serializedPK []byte
Expand All @@ -338,82 +367,88 @@ func verifyBitcoinSignature(sig, msg, addr string) {

address, err := btcutil.NewAddressPubKey(serializedPK, &chaincfg.MainNetParams)
if err != nil {
log.Fatal("Address recovery failed")

return fmt.Errorf("address recovery from pubkey failed: %w", err)
}

if address.EncodeAddress() != addr {
log.Fatal("invalid signature")
return fmt.Errorf("address mismatch, expected %s, got %s", addr, address.EncodeAddress())
}

return nil
}

func Prop29Migration(authGenesis *authtypes.GenesisState, bankGenesis *banktypes.GenesisState, distrGenesis *distr.GenesisState) (authtypes.GenesisState, banktypes.GenesisState, distr.GenesisState) {

fundRecovery := validateFundRecovery()

recoveryAccounting := sdk.NewInt64Coin("uatom", 0)

emptyCoins := []sdk.Coin{}

distModuleAccount := authtypes.NewModuleAddress(distr.ModuleName)

//Set All Source Addresses to Zero and accumulate the total funds being moved
// zero out all source addresses balances and accumulate the total funds being moved
for i, balance := range bankGenesis.Balances {
_, isSourceAddress := fundRecovery.IsSourceAddress(balance.Address)
if isSourceAddress {
if len(balance.Coins) > 1 {
log.Fatal("Expected all balances to contain only 1 denom during the migration")
}
// Accumulate all the coins removed from the balances
recoveryAccounting = recoveryAccounting.Add(balance.Coins[0])
// Zero out the Balance
bankGenesis.Balances[i].Coins = emptyCoins
if !isSourceAddress {
continue
}

if len(balance.Coins) > 1 {
log.Fatal("expected all balances to contain only 1 denom during the migration")
}
// accumulate all the coins removed from the balances into the pool
recoveryAccounting = recoveryAccounting.Add(balance.Coins[0])
// Empty the source address balance
bankGenesis.Balances[i].Coins = emptyCoins
}

// migrate the balances to the the destination addresses
for i, balance := range bankGenesis.Balances {
index, isDestAddress := fundRecovery.IsDestAddress(balance.Address)
if isDestAddress {
recoveryAccounting = recoveryAccounting.Sub(fundRecovery[index].destBalance)
bankGenesis.Balances[i].Coins = bankGenesis.Balances[i].Coins.Add(fundRecovery[index].destBalance)
fundRecovery[index].destBalance = sdk.NewInt64Coin("uatom", 0)
if !isDestAddress {
continue
}

// transfer coins from the atom recovery pool to the dest address
recoveryAccounting = recoveryAccounting.Sub(fundRecovery[index].destBalance)
bankGenesis.Balances[i].Coins = bankGenesis.Balances[i].Coins.Add(fundRecovery[index].destBalance)
fundRecovery[index].destBalance = sdk.NewInt64Coin("uatom", 0)
}

// add the balances to the bank genesis
bankGenesis.Balances = append(bankGenesis.Balances, fundRecovery.GetRemainingBalances()...)
Copy link
Collaborator

Choose a reason for hiding this comment

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

My only concern here is that a balance uniqueness is not validated on the bank genesis validation


accs, err := authtypes.UnpackAccounts(authGenesis.Accounts)
if err != nil {
log.Fatal("Could not unpack genesis account")
log.Fatalf("could not unpack genesis accounts: %s", err.Error())
}

// add the accounts with positive balance to the genesis accounts
for _, addr := range fundRecovery.GetRemainingAccounts() {
recoveryAccount := authtypes.NewBaseAccount(sdk.AccAddress(addr.Bytes()), nil, 0, 0)

accs = append(accs, recoveryAccount)

}

genAccs, err := authtypes.PackAccounts(accs)
if err != nil {
log.Fatal(err)
log.Fatalf("could not pack genesis accounts: %s", err.Error())
}

authGenesis.Accounts = genAccs

// subtract the coins from the addresses with positive balances from the recovery accounting
for _, balance := range fundRecovery.GetRemainingBalances() {
recoveryAccounting = recoveryAccounting.Sub(balance.Coins[0])
}

// Add the remaining ATOMs to the fee pool by adding them to distribution modules
for i, balance := range bankGenesis.Balances {
if distModuleAccount.String() == balance.Address {
bankGenesis.Balances[i].Coins = bankGenesis.Balances[i].Coins.Add(recoveryAccounting)

distrGenesis.FeePool.CommunityPool = distrGenesis.FeePool.CommunityPool.Add(sdk.NewDecCoinFromCoin(recoveryAccounting))

recoveryAccounting = sdk.NewInt64Coin("uatom", 0)
if distModuleAccount.String() != balance.Address {
continue
}

// add coins to the community pool bank and distr balances
bankGenesis.Balances[i].Coins = bankGenesis.Balances[i].Coins.Add(recoveryAccounting)
distrGenesis.FeePool.CommunityPool = distrGenesis.FeePool.CommunityPool.Add(sdk.NewDecCoinFromCoin(recoveryAccounting))
break
}

return *authGenesis, *bankGenesis, *distrGenesis
Expand Down
Loading