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

USDC: @celo/connect + web3js demo #3

Merged
merged 9 commits into from
Feb 21, 2024
Merged

Conversation

arthurgousset
Copy link
Contributor

@arthurgousset arthurgousset commented Feb 16, 2024

Copy link

socket-security bot commented Feb 16, 2024

New dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
npm/@celo/connect@5.1.2 Transitive: environment, eval, filesystem, network, shell, unsafe +356 47.4 MB app-tooling
npm/@celo/wallet-local@5.1.2 Transitive: environment, eval, filesystem, network, shell, unsafe +363 50.1 MB app-tooling
npm/@ethereumjs/common@2.6.5 Transitive: environment, filesystem +43 12.4 MB holgerd77
npm/@ethersproject/transactions@5.7.0 None +18 1.1 MB ricmoo
npm/@noble/curves@1.3.0 None +1 2.15 MB paulmillr
npm/@noble/hashes@1.3.3 None 0 761 kB paulmillr
npm/@noble/secp256k1@1.7.1 None 0 111 kB paulmillr
npm/@sindresorhus/is@4.6.0 None 0 57.5 kB sindresorhus
npm/@types/bn.js@5.1.5 None +2 4.09 MB types
npm/@types/cacheable-request@6.0.3 None +5 4.1 MB types
npm/abortcontroller-polyfill@1.7.5 network 0 94.6 kB molsson
npm/assert-plus@1.0.0 environment 0 11.4 kB pfmooney
npm/base-x@3.0.9 None +1 41.4 kB junderw
npm/bn.js@5.2.1 None 0 99 kB fanatid
npm/buffer@5.7.1 None +2 98.9 kB feross
npm/call-bind@1.0.7 Transitive: eval +11 227 kB ljharb
npm/cipher-base@1.0.4 None +2 44 kB cwmma
npm/combined-stream@1.0.8 None +1 19.5 kB alexindigo
npm/content-type@1.0.5 None 0 10.5 kB dougwilson
npm/create-hash@1.2.0 Transitive: environment +10 248 kB cwmma
npm/create-hmac@1.1.7 Transitive: environment +11 254 kB cwmma
npm/d@1.0.1 Transitive: eval +7 665 kB medikoo
npm/defer-to-connect@2.0.1 None 0 5.44 kB szmarczak
npm/elliptic@6.5.4 None +6 199 kB indutny
npm/es5-ext@0.10.62 eval +7 665 kB medikoo
npm/es6-symbol@3.1.3 Transitive: eval +7 665 kB medikoo
npm/eth-lib@0.2.8 Transitive: network +31 579 kB maiavictor
npm/ethereum-cryptography@1.2.0 None +5 1.22 MB paulmillr
npm/ethereumjs-util@7.1.5 Transitive: environment, filesystem +41 9.82 MB holgerd77
npm/extsprintf@1.3.0 None 0 22.8 kB dap
npm/get-intrinsic@1.2.4 eval +5 124 kB ljharb
npm/graceful-fs@4.2.11 environment, filesystem 0 32.5 kB isaacs
npm/has-tostringtag@1.0.2 None +1 38.2 kB ljharb
npm/hash.js@1.1.7 None +2 47.2 kB indutny
npm/inherits@2.0.4 None 0 3.96 kB isaacs
npm/is-typedarray@1.0.0 None 0 4.41 kB hughsk
npm/js-sha3@0.8.0 None 0 52.9 kB emn178
npm/mime-types@2.1.35 None +1 224 kB dougwilson
npm/minimalistic-assert@1.0.1 None 0 1.55 kB cwmma
npm/minipass@2.9.0 None +2 83.4 kB isaacs
npm/multihashes@0.4.21 None +7 1.01 MB hugomrdias
npm/node-gyp-build@4.8.0 environment, filesystem 0 13.4 kB mafintosh
npm/object-assign@4.1.1 None 0 5.49 kB sindresorhus
npm/once@1.4.0 None +1 7.01 kB isaacs
npm/punycode@2.3.1 None 0 33.5 kB google-wombot
npm/resolve-alpn@1.2.1 network 0 4.64 kB szmarczak
npm/ripemd160@2.0.2 Transitive: environment +6 196 kB dcousens
npm/safe-buffer@5.2.1 None 0 32.1 kB feross
npm/safer-buffer@2.1.2 None 0 42.3 kB chalker
npm/scrypt-js@3.0.1 None 0 226 kB ricmoo
npm/sha.js@2.4.11 None +2 67.1 kB dcousens
npm/tweetnacl@0.14.5 None 0 174 kB dchest
npm/unpipe@1.0.0 None 0 4.31 kB dougwilson
npm/vary@1.1.2 None 0 8.75 kB dougwilson
npm/web3-utils@1.10.4 Transitive: network +17 4.14 MB jdevcs
npm/web3@1.10.4 Transitive: environment, eval, filesystem, network, shell, unsafe +341 42.5 MB jdevcs
npm/which-typed-array@1.1.14 Transitive: eval +16 348 kB ljharb
npm/xhr-request@1.1.0 Transitive: network +21 199 kB mattdesl
npm/xhr@2.6.0 None +7 115 kB naugtur
npm/yallist@3.1.1 None 0 14.8 kB isaacs

View full report↗︎

Copy link

socket-security bot commented Feb 16, 2024

🚨 Potential security issues detected. Learn more about Socket for GitHub ↗︎

To accept the risk, merge this PR and you will not be notified again.

Alert Package NoteSource
Protestware/Troll package npm/es5-ext@0.10.62
  • Note: This package prints a protestware console message on install regarding Ukraine for users with Russian language locale
Install scripts npm/es5-ext@0.10.62
  • Install script: postinstall
  • Source: node -e "try{require('./_postinstall')}catch(e){}" || exit 0
Native code npm/utf-8-validate@5.0.10
Native code npm/secp256k1@4.0.3
Native code npm/keccak@3.0.4
Install scripts npm/web3-bzz@1.10.0
  • Install script: postinstall
  • Source: echo "WARNING: the web3-bzz api will be deprecated in the next version"
Install scripts npm/web3-shh@1.10.0
  • Install script: postinstall
  • Source: echo "WARNING: the web3-shh api will be deprecated in the next version"
Native code npm/bufferutil@4.0.8
Install scripts npm/web3-bzz@1.10.4
  • Install script: postinstall
  • Source: echo "WARNING: the web3-bzz api will be deprecated in the next version"
Install scripts npm/web3-shh@1.10.4
  • Install script: postinstall
  • Source: echo "WARNING: the web3-shh api will be deprecated in the next version"
Install scripts npm/web3@1.10.0
  • Install script: postinstall
  • Source: echo "Web3.js 4.x alpha has been released for early testing and feedback. Checkout doc at https://docs.web3js.org/ "
Install scripts npm/web3@1.10.4
  • Install script: postinstall
  • Source: echo "Web3.js 4.x alpha has been released for early testing and feedback. Checkout doc at https://docs.web3js.org/ "

View full report↗︎

Next steps

What is protestware?

This package is a joke, parody, or includes undocumented or hidden behavior unrelated to its primary function.

Consider that consuming this package my come along with functionality unrelated to its primary purpose.

What is an install script?

Install scripts are run when the package is installed. The majority of malware in npm is hidden in install scripts.

Packages should not be running non-essential scripts during install and there are often solutions to problems people solve with install scripts that can be run at publish time instead.

What's wrong with native code?

Contains native code which could be a vector to obscure malicious code, and generally decrease the likelihood of reproducible or reliable installs.

Ensure that native code bindings are expected. Consumers may consider pure JS and functionally similar alternatives to avoid the challenges and risks associated with native code bindings.

Take a deeper look at the dependency

Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev.

Remove the package

If you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency.

Mark a package as acceptable risk

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of ecosystem/package-name@version specifiers. e.g. @SocketSecurity ignore npm/foo@1.0.0 or ignore all packages with @SocketSecurity ignore-all

  • @SocketSecurity ignore npm/es5-ext@0.10.62
  • @SocketSecurity ignore npm/utf-8-validate@5.0.10
  • @SocketSecurity ignore npm/secp256k1@4.0.3
  • @SocketSecurity ignore npm/keccak@3.0.4
  • @SocketSecurity ignore npm/web3-bzz@1.10.0
  • @SocketSecurity ignore npm/web3-shh@1.10.0
  • @SocketSecurity ignore npm/bufferutil@4.0.8
  • @SocketSecurity ignore npm/web3-bzz@1.10.4
  • @SocketSecurity ignore npm/web3-shh@1.10.4
  • @SocketSecurity ignore npm/web3@1.10.0
  • @SocketSecurity ignore npm/web3@1.10.4

@arthurgousset
Copy link
Contributor Author

arthurgousset commented Feb 16, 2024

Decent progress on the USDC demo front. Currently, I'm blocked on an example that exclusively uses @celo/connect and web3js to make a fee currency transaction.

Generic ERC20 transfers (type: EIP1559) work well (example transaction implemented in 0c5a0b7), but I couldn't yet work out how to correctly pass a feeCurrency field and send a CIP64 transaction.

@arthurgousset arthurgousset self-assigned this Feb 16, 2024
@arthurgousset arthurgousset changed the title Web3js demo USDC: @celo/connect + web3js demo Feb 16, 2024
@arthurgousset
Copy link
Contributor Author

arthurgousset commented Feb 20, 2024

copy/paste insight from @aaronmgdr

ok so here is the issue.

in order to sign a cip64 tx a Wallet that is a subclass of WalletBase must be used as that is where the actual code for rlp encode and sign is. in the setup you have its using the wallet on web3. which only knows eip1559

this line https://github.com/celo-org/developer-tooling/blob/679ef0c607500a69f2d1d684740632202ab79ad2/packages/sdk/connect/src/connection.ts#L72

if it were not commented out would default the wallet on connection to a LocalWallet. instead of a ReadOnly Wallet. we should either uncomment that or make it so calling connection.addAccount creates an LocalWallet instance (which would make more sense to me)

@arthurgousset
Copy link
Contributor Author

arthurgousset commented Feb 20, 2024

I'm wondering what purpose the ReadOnlyWallet interface serves.

export interface ReadOnlyWallet {
    getAccounts: () => Address[];
    removeAccount: (address: Address) => void;
    hasAccount: (address?: Address) => boolean;
    signTransaction: (txParams: CeloTx) => Promise<EncodedTransaction>;
    signTypedData: (address: Address, typedData: EIP712TypedData) => Promise<string>;
    signPersonalMessage: (address: Address, data: string) => Promise<string>;
    decrypt: (address: Address, ciphertext: Buffer) => Promise<Buffer>;
    computeSharedSecret: (address: Address, publicKey: string) => Promise<Buffer>;
}

Source: connect > wallet.ts

Why is it called "ReadOnly" if you can sign a transaction with a class that implements it?

The Connection class uses this interface:

/**
 * Connection is a Class for connecting to Celo, sending Transactions, etc
 * @param web3 an instance of web3
 * @optional wallet a child class of {@link WalletBase}
 * @optional handleRevert sets handleRevert on the web3.eth instance passed in
 */
export class Connection {
    // ...
    constructor(readonly web3: Web3, public wallet?: ReadOnlyWallet, handleRevert = true) {
    // ...
    // TODO: Add this line with the wallets separation completed
    // this.wallet = _wallet ?? new LocalWallet()
    this.config.from = web3.eth.defaultAccount ?? undefined

Source: connect > connection.ts

Also curious how _wallet and LocalWallet() are defined.

@arthurgousset
Copy link
Contributor Author

arthurgousset commented Feb 20, 2024

Wondering if LocalWallet() is a web3.js specific concept.

Local wallets are an in-memory wallet that can hold multiple accounts. Wallets are a convenient way to sign and send transactions in web3.js

Source: https://docs.web3js.org/guides/wallet/local_wallet/

It's not obvious where web3.js stops and Celo-specific constructs start (e.g. ReadOnlyWallet).

@aaronmgdr
Copy link
Member

Wondering if LocalWallet() is a web3.js specific concept.

Local wallets are an in-memory wallet that can hold multiple accounts. Wallets are a convenient way to sign and send transactions in web3.js

Source: https://docs.web3js.org/guides/wallet/local_wallet/

It's not obvious where web3.js stops and Celo-specific constructs start (e.g. ReadOnlyWallet).

while web3 has wallets. you must use the celo wallets to get the signing happening -- see the wallets directory in the sdks folder of developertooling repo.

@aaronmgdr
Copy link
Member

aaronmgdr commented Feb 20, 2024

Also curious how _wallet and LocalWallet() are defined.

_wallet i think comes from the params above (which is now just wallet)

LocalWallet would be imported from @celo/wallet-local

@aaronmgdr
Copy link
Member

Why is it called "ReadOnly" if you can sign a transaction with a class that implements it?

yeah i cant answer that one. seems like it should be have a better name

@arthurgousset
Copy link
Contributor Author

Also curious how _wallet and LocalWallet() are defined.

_wallet i think comes from the params above (which is now just wallet)

LocalWallet would be imported from @celo/wallet-local

Thanks, this helped a lot. I used LocalWallet in c9b9e3f.

At the moment, the script is stuck and doesn't timeout or complete. I'll debug that next

web3.ts Show resolved Hide resolved
@arthurgousset
Copy link
Contributor Author

arthurgousset commented Feb 20, 2024

(for future reference)
Also noting that type: 'cip64' in the Connection.sendTransaction did not work although, the type field should accept 'cip64'

export type TransactionTypes = 'eip1559' | 'celo-legacy' | 'cip42' | 'cip64'

Source: connect > types.ts

const transactionReceipt = await connection
        .sendTransaction({
            from: sender,
            to: CONTRACT_ADDRESS,
            gas: 51925, // TODO: implement gas estimation
            maxPriorityFeePerGas: web3.utils.toWei("10", "gwei"),
            maxFeePerGas: web3.utils.toWei("10", "gwei"),
            feeCurrency: "0x874069Fa1Eb16D44d622F2e0Ca25eeA172369bC1", // cUSD fee currency
            data: transactionObject.encodeABI(),
+           type: 'cip64'
        })

The error is: Cannot convert string to Uint8Array. only supports 0x-prefixed hex strings and this string was given: cip64

$ yarn ts-node web3.ts 
yarn run v1.22.19
$ /Users/arthur/Documents/celo-org/feecurrency/node_modules/.bin/ts-node web3.ts
Initiating fee currency transaction...
/Users/arthur/Documents/celo-org/feecurrency/node_modules/web3-eth-accounts/src/common/utils.ts:344
                        throw new Error(
         ^
Error: Cannot convert string to Uint8Array. only supports 0x-prefixed hex strings and this string was given: cip64
    at toUint8Array (/Users/arthur/Documents/celo-org/feecurrency/node_modules/web3-eth-accounts/src/common/utils.ts:344:10)
    at Function.typeToInt (/Users/arthur/Documents/celo-org/feecurrency/node_modules/web3-eth-accounts/src/tx/transactionFactory.ts:43:48)
    at Function.fromTxData (/Users/arthur/Documents/celo-org/feecurrency/node_modules/web3-eth-accounts/src/tx/transactionFactory.ts:68:37)
    at /Users/arthur/Documents/celo-org/feecurrency/node_modules/web3-eth/src/utils/prepare_transaction_for_signing.ts:145:28
    at Generator.next (<anonymous>)
    at fulfilled (/Users/arthur/Documents/celo-org/feecurrency/node_modules/web3-eth/lib/commonjs/utils/prepare_transaction_for_signing.js:21:58)
    at processTicksAndRejections (node:internal/process/task_queues:95:5)
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

Might be a different reason, but keeping track of this here in case.

This fixes a bug with gas estimation. The `@celo/connect` library is calling a method from web3js.

```ts
estimateGas = async (
    tx: CeloTx,
    gasEstimator: (tx: CeloTx) => Promise<number> = this.web3.eth.estimateGas,
    caller: (tx: CeloTx) => Promise<string> = this.web3.eth.call
  ): Promise<number>
```

Source: https://github.com/celo-org/developer-tooling/blob/a884b24a242829a9425580e0b75f40526228838f/packages/sdk/connect/src/connection.ts#L358

Because of the version mismatch, the call fails silently.
- gas estimation is done automatically
- with web3js@1.10, ERC20 ABI has to be of type `AbiItem[]`
@arthurgousset
Copy link
Contributor Author

arthurgousset commented Feb 21, 2024

Hint: Learnt that prepending a Node.js command with DEBUG=* executes the Node.js script with verbose output in the console.

For example:

$ DEBUG=* yarn ts-node web3.ts

@arthurgousset
Copy link
Contributor Author

With the updated script, @celo/connect + web3 (incl. @celo/wallet-local) successfully makes a CIP64 transaction.
For example: https://alfajores.celoscan.io/tx/0x0cbac87c455a0cf9a1cb6bd340f1332d45eb767a570e901d72b386fb58790f79

@arthurgousset arthurgousset marked this pull request as ready for review February 21, 2024 17:20
@arthurgousset arthurgousset merged commit d0cd7c8 into main Feb 21, 2024
2 of 3 checks passed
@arthurgousset arthurgousset deleted the arthurgousset/demos branch February 21, 2024 17:22
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