brokenbydefault / Nanollet

The unique ultra-light wallet for Nano/Raiblocks with amazing features, written in Golang.
https://nanollet.org
MIT License
53 stars 7 forks source link

opencap alias support #17

Closed wagslane closed 6 years ago

wagslane commented 6 years ago

I added support for OpenCAP aliases. I tested to make sure the addresses are pulled in correctly. Obviously I raised an issue where my wallet isn't creating a receive block but that shouldn't change this PR.

I'm not sure if I messed up how you wanted to organize your code or not, let me know.

I think those generated files should NOT be in the repo. I was talking about the files below that should be in the repo:

In order to test, make an alias at https://ogdolo.com

inkeliz commented 6 years ago

I not sure if it should be inside Util, or have they own folder, like Nanollet/OpenCAP. Seems that the http.Get doesn't set a timeout, maybe should create a http.Client{Timeout: ... } instead of use the DefaultClient?

I think this ifs:

addr, err := Util.LookupAlias(addrOrAlias)
if err != nil && err.Error() != "Invalid alias provided" {
    DOM.UpdateNotification(w, err.Error())
    return
} else if err != nil {
    addr = addrOrAlias
}

https://github.com/lane-c-wagner/Nanollet/blob/80633f81fc26efa62ec1fcf6299e8a28765d796d/GUI/App/nanollet.go#L67-L73

Something like a switch:

var address Wallet.Address
switch {
case Wallet.Address(addrOrAlias).IsValid():
    address = Wallet.Address(addrOrAlias)
case opencap.ValidateAlias(addrOrAlias)
    addr, err := Util.LookupAlias(addrOrAlias)
    append(errs, err)
    address = Wallet.Adress(addr)
default:
        err = ...
}

If LookupAlias returns a Wallet.Address, which is impossible inside Util, we can use directly address, error = AnotherLib.LookupAlias(addrOrAlias).

At least for me it's more easy to understand what is doing on, if it's already a valid address it is used as a address. If a new option becomes available is just a new case, in most of cases.

I'm testing it right now.

inkeliz commented 6 years ago

Should we add KeyPinning for already-know OpenCAP severs?

In the past, when uses a server (instead of connect to the nodes). So, the wallet have a hardcoded public-key, it prevents a MITM when the client trust in some insecure certificate authorities. In this case it was set in:

https://github.com/brokenbydefault/Nanollet/blob/5de08955ccb4672e12c64721ff914b62b502680d/RPC/Connectivity/socket.go#L30

Then it's used in the TlsConfig of the Dialer:

https://github.com/brokenbydefault/Nanollet/blob/5de08955ccb4672e12c64721ff914b62b502680d/RPC/Connectivity/socket.go#L48-L52

I think should possible to do a Key Pinning using the Config, but I'm not sure how long should this key live. If the TLS key rotates too frequently, it's useless. If the key is intended to be replaced together with the certificate (every "certificate renew" uses a new key) we can set a lifetime based on the certificate.

wagslane commented 6 years ago

I don't think we can do anything for "known opencap servers" because the point of OpenCAP is to be an opencap protocol and allow anyone to run a server. Ogdolo just happens to be the only one running now because its a new protocol. In the future there will hopefully be more servers than we can count!

wagslane commented 6 years ago

I not sure if it should be inside Util, or have they own folder, like Nanollet/OpenCAP. Seems that the http.Get doesn't set a timeout, maybe should create a http.Client{Timeout: ... } instead of use the DefaultClient?

I think this ifs:

addr, err := Util.LookupAlias(addrOrAlias)
if err != nil && err.Error() != "Invalid alias provided" {
  DOM.UpdateNotification(w, err.Error())
  return
} else if err != nil {
  addr = addrOrAlias
}

https://github.com/lane-c-wagner/Nanollet/blob/80633f81fc26efa62ec1fcf6299e8a28765d796d/GUI/App/nanollet.go#L67-L73

Something like a switch:

var address Wallet.Address
switch {
case Wallet.Address(addrOrAlias).IsValid():
  address = Wallet.Address(addrOrAlias)
case opencap.ValidateAlias(addrOrAlias)
  addr, err := Util.LookupAlias(addrOrAlias)
  append(errs, err)
  address = Wallet.Adress(addr)
default:
        err = ...
}

If LookupAlias returns a Wallet.Address, which is impossible inside Util, we can use directly address, error = AnotherLib.LookupAlias(addrOrAlias).

At least for me it's more easy to understand what is doing on, if it's already a valid address it is used as a address. If a new option becomes available is just a new case, in most of cases.

I'm testing it right now.

LookupAlias returns the string representation of a Nano address and an error, I can change that though

inkeliz commented 6 years ago

I working on it right now. 👍

wagslane commented 6 years ago

Cool, I'll just let you refactor. If need any input let me know

wagslane commented 6 years ago

Also, if you have any generic OpenCAP questions, we are available in our discord server: https://discord.gg/FTMCBE8