New nfts driver #11433
New nfts driver #11433
Conversation
Offer and trade manager generalization
|
This pull request introduces 8 alerts when merging 297767f into 57a2415 - view on LGTM.com new alerts:
|
|
This pull request introduces 1 alert when merging c470401 into 57a2415 - view on LGTM.com new alerts:
|
|
This pull request introduces 1 alert when merging 150f244 into 57a2415 - view on LGTM.com new alerts:
|
|
This pull request introduces 1 alert when merging 8a32eee into 57a2415 - view on LGTM.com new alerts:
|
|
This pull request introduces 1 alert when merging b69323e into 57a2415 - view on LGTM.com new alerts:
|
|
This pull request introduces 1 alert when merging 4df1f46 into 57a2415 - view on LGTM.com new alerts:
|
wouldn't full solution still contain those conditions as it won't get run to get them excluded? (I extract the conditions from the inner_solution in the solution) |
good point, fixed |
|
This pull request introduces 1 alert when merging 61bb87f into 57a2415 - view on LGTM.com new alerts:
|
| elif address is None: | ||
| puzzle_hash = await nft_wallet.standard_wallet.get_new_puzzlehash() | ||
| else: | ||
| puzzle_hash = address |
There was a problem hiding this comment.
Is it possible to hit this condition? I would think that the address could only be either a string or None to successfully be marshalled through the RPC interface. I would raise an exception in this case.
There was a problem hiding this comment.
previous RPC was sending over bytes to pass puzzle hash and string to pass address, I added the None condition but it doesn't need to be there, it's just the way I was writing unit tests for RPC
There was a problem hiding this comment.
I see. So it sounds like there are some RPC callers (maybe only in the tests) that don't go through JSON.
| for nft in nfts: | ||
| nft_info_list.append(get_nft_info_from_puzzle(nft.full_puzzle, nft.coin)) | ||
| return {"wallet_id": wallet_id, "success": True, "nft_list": nft_info_list} | ||
| uri = get_uri_list_from_puzzle(nft.full_puzzle) |
There was a problem hiding this comment.
This will break the GUI integration, as the frontend is expecting an array of NFTInfo objects, but we can fix it in a follow-up PR.
| # Now we must have unused public keys | ||
| unused = await self.puzzle_store.get_unused_derivation_path() | ||
| assert unused is not None | ||
| self.log.debug("Fetching derivation record for: %s %s %s", unused, wallet_id, hardened) |
There was a problem hiding this comment.
nit: we don't seem to be entirely consistent with format strings in the codebase, but generally f-strings are preferred.
There was a problem hiding this comment.
f-strings are good, but the problem with using f-string formatting in logging is those variables get evaluated even if logging level is above. See this.
No description provided.