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

shakedex: use derive path for private key #416

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

chikeichan
Copy link
Contributor

@chikeichan chikeichan commented Oct 20, 2021

this is harder than i thought - mostly because of all the edge around rescanning auctions.

This probably take some time for review and won't make it into 0.9.0-rc.1 tomorrow. let's see if we can include this before 0.9.0 turn stable.

Includes:

  • Generate script address using standardized branch path
  • Add rescan listing
  • Add rescan fills by name
  • Add loading state in UI
  • Add HIP-0009 for deriving key for script address

recanauction

requesting review from @kurumiimari @pinheadmz


if (!sAuction || sAuction.lockingTxHash !== owner.hash) return;

const lockingPublicKey = sAuction.publicKey;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

is it possible to recover this based on on-chain data @pinheadmz ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Once HIP-9 is implemented in hsd wallet, it should be able to recover HIP-1 addresses from the blockchain just like it recovers receive and change addresses.

@pinheadmz
Copy link
Contributor

I plan on reviewing this in depth but the first comment I have is this: I think we should try to limit Bob just to UI functions and keep wallet / node features exclusively in hsd (this applies to rescan stuff / batch TXs, too).

Especially now that you have proposed HIP-0009 formally we can begin working on using the wallet software we already have to recover HIP-9 addresses, etc. I understand that may slow down progress, but I think that is ok, it will certainly get better review and test coverage that way and opens the door for other HIP-1 implementations to build on hsd. Are there any other concerns with opening this kind of PR to hsd? We could discuss in an issue or something.

@pinheadmz
Copy link
Contributor

You might want to follow / review handshake-org/hsd#639 as well. @Anunayj is also interested in a HIP-9 standard custom script and we are working on adding custom script signing to hsd wallet.

@chikeichan
Copy link
Contributor Author

I plan on reviewing this in depth but the first comment I have is this: I think we should try to limit Bob just to UI functions and keep wallet / node features exclusively in hsd (this applies to rescan stuff / batch TXs, too).

Especially now that you have proposed HIP-0009 formally we can begin working on using the wallet software we already have to recover HIP-9 addresses, etc. I understand that may slow down progress, but I think that is ok, it will certainly get better review and test coverage that way and opens the door for other HIP-1 implementations to build on hsd. Are there any other concerns with opening this kind of PR to hsd? We could discuss in an issue or something.

yea i am open to it. so i think we would need...

  • add support for custom branch to the bloomfilter in wallet.rescan, maybe enable by a hip9 (uint32[]) config to WalletDB?
  • (unsure?) return hip9 attributes on tx and domain object from API. this help UI with identifying non-default TX and Domains. In the case of shakedex, it is used to show listings and fills. we could maintain a reverse index of addresses to hip9 for more efficient look up

@pinheadmz
Copy link
Contributor

Yeah so if you look through wallet/account.js in hsd theres a bunch of functions like deriveChange() deriveReceive() and syncDepth() which calls both of those. Then when you get an account object it has properties like:

  "receiveAddress": "rs1qy9uplxpt5cur32rw3zmyf8e7tp87w8slly2fms",
  "changeAddress": "rs1q30ppv5gyrwpy4wyk0v6uzawxygdtvrpux8yrg2",

So the naive way to go forward with this would be duplicate every single one of those and change it to deriveSakedex() and shakedexAddress: ... and all that... but we'll have to do that for every single HIP9 proposal forever.

What we may want to do is add a new wallet type or account type similar to Bitcoin's descriptor wallets where arbitrary scripts with public-key placeholders can be imported. The wallet will then proceed as normal: generate the lookahead amount of addresses and add them to the bloom filter for re-scanning, etc.

I think the hsd wallet should not necessarily be required to know how to spend from those outputs, I'm not sure about that yet. As long as the wallet has good API for signing arbitrary scripts we should be ok with minimal additions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants