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

feat: no buffer req. rewrite #389

Merged
merged 98 commits into from
Feb 5, 2024
Merged

feat: no buffer req. rewrite #389

merged 98 commits into from
Feb 5, 2024

Conversation

TateB
Copy link
Collaborator

@TateB TateB commented Sep 15, 2023

No description provided.

@ukstv
Copy link

ukstv commented Sep 17, 2023

@TateB 2 cents from an almost bystander. The code becomes much-much easier to leverage https://github.com/paulmillr/scure-base and Coder interface there. 80% of the plumbing goes away with that.

@TateB
Copy link
Collaborator Author

TateB commented Sep 17, 2023

@TateB 2 cents from an almost bystander. The code becomes much-much easier to leverage https://github.com/paulmillr/scure-base and Coder interface there. 80% of the plumbing goes away with that.

completely forgot about that, thanks! will use it

edit: looks like the lib is quite slow, might not use it

@ukstv
Copy link

ukstv commented Sep 18, 2023

@TateB Is this library used for high-performance processing? IMO, its main use case is on UI where speed is not that important. Size is quite important though.

Anyhow, that's what pissed me off about the current state of the library. I was disappointed enough to do a rewrite myself. Considering how PRs are merged here, it's a separate repo: https://github.com/ukstv/ens-address-encoder maybe you'll find it useful.

@ukstv
Copy link

ukstv commented Sep 18, 2023

Rooting for this to land in prod, anyway.

@TateB
Copy link
Collaborator Author

TateB commented Sep 18, 2023

Is this library used for high-performance processing? IMO, its main use case is on UI where speed is not that important. Size is quite important though.

not internally, but in the future it's definitely possible. performance isn't really a focus as much as it is a nice to have, but using the @scure/base lib (when most of the funcs being used from it were already written) led to a 5-10x performance decrease.

your rewrite looks great! if you add a LICENSE to the repo that allows me to use some of the code, i definitely might

@talentlessguy
Copy link

talentlessguy commented Sep 27, 2023

I can't decode a BTC address:

import { createPublicClient, http } from 'viem'
import { mainnet } from 'viem/chains'
import { getCoderByCoinType } from '@ensdomains/address-encoder'

const td = new TextDecoder()

const publicClient = createPublicClient({
  transport: http('https://rpc.ankr.com/eth'),
  chain: mainnet,
})

for (const coinType of resolver.coinTypes) {
  const hexAddress = await publicClient.getEnsAddress({
    coinType: parseInt(coinType),
    name,
  })

  const coin = getCoderByCoinType(parseInt(coinType))

  if (hexAddress) {
    data.coins.push({
      type: coin.name,
      address:
        coin.coinType === 60
          ? hexAddress
          : td.decode(coin.decode(hexAddress.slice(2))), // <- the error happens here
    })
  }
}

it parses ethereum fine but on btc this happens:

⨯ Error: Invalid checksum

the hex address (returned by viem) is 0x001499e3452752d781c06aa4fb828e85ea7d3b57ff1b. I tried slicing 0x, and keeping it, same result. Am I doing something wrong?

(it should decode to bc1qn8352f6j67quq64ylwpgap0205a40lcmurvxwq)

README.md Outdated Show resolved Hide resolved
@TateB TateB merged commit 03aa74e into master Feb 5, 2024
2 checks passed
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.

5 participants