chia-blockchain icon indicating copy to clipboard operation
chia-blockchain copied to clipboard

Calculate royalty details locally if the NFT mint is part of an NFT offer

Open greimela opened this issue 6 months ago • 6 comments

Purpose:

This fix enables the Chia wallet to take an offer where the NFT mint is part of the spend bundle.

Current Behavior:

The royalty calculation fails, since it wants to look up the latest NFT spend on chain, which it can't find.

New Behavior:

If the eve coin of an NFT is part of the spend, it will use that to figure out the royalty percentage and address, instead of asking the full node.

Testing Notes:

As far as I could tell, there is no unit test for an XCH/NFT trade on this level.

For manual testing, here is an offer that mints and trades an NFT on testnet11: offer1qqr83wcuu2rykcmqvpsxzgqqemhmlaekcenaz02ma6hs5w600dhjlvfjn477nkwz369h88kll73h37fefnwk3qqnz8s0lle04zh8se9ldh0venenns820n90dtl39xxrc20da7uh049cqsap5dkzkvz89hs85v8hejakl8jkrllqd6d2ckwn3xzlltsa80gmsu8haht3hajrmrmyl8ljdeshvqaauzdn93dt0aedsaruqm44svk9xre0exuzs525lusx6xk8t9067adhzdld0l0884vd6463k9vm4e7t480320ulv7r5whfwp7uk87fs028a6qs3dr2rrshzp6j6mkeylc0jn8u2gz95z35sxasgyxksena37adulsl5vwcwt5z95mrjllreqswv7nvcqz4lqczgwzw7u9qkjlxr43psc9vprj8wck9qwwu4r7e3ppferx9qse9s8w3uv3w9j87wzar4xsc9c7qfggx2xdv8d700qm4w6xyrt2j2p2z3gp43egw47zc9aqqd0pcqfw2lvqp7eq4qwemmqramvy6y0tvsn234v3p4glsnvknqwvp49hcyu9u0egmz2nhehqxrweql624rur7hwte7hpme4ymd6wwjfa7agl8z0az8nrzg5k7afeltrftx8e2dn7c0549fuqgfqlftaegm45umf0xwe2ht36j577az29uhlyknj77swcj4fkgzmlu7ze07y997c8qv7m7lvmtqef4k092uk5kdjexapupl7k8xexr6pj5y2cwuhya6vlte66xakl7vpckddax0vhesxrd77l8emswfd6epqxvc25r9l7atelw5zu7e4ett0l7deh9q4td60nhvm2m8dugyn0eav2uhva0t4hsd24m8fadtet66kx9yg02w7tecw80wj497cht3d6v2n7ds7pg4mdkm4fawqe2uv98m3el78j6lstxyej6lam0ldw83z3nk98m6c5rqnla9cumft83lyu84234shsqjk6mzlsp9ttcek8sh04y2cln09h5ps7p9lp8um8qz8ujkae9wrmdz55ynkuk7mdkcxfn5dre9acxwhnal45345hs3u6fngjv228twa60e4k060pxmac64fhamwnn27t935zpfyq2p82x9rquzxgnjydf9lyc92flsp903v9j94fpkxf9z78v6r7xcrzgjgaewfsujrnucsj5wpvxwl7m2zkemul6ew0a9n7rhk8eyec0rsunh77h7n8l687uxg5449jc7stdekfur6n0e0wga44u4d79xc2lwqwwl7g4drqmleekhf8r5wmnxzth0ljmcgrxp2tyr8r2wsk85apfrvfl8g2suycuxffkzy95ekuz4k6q9p62pvs6cw5nc338vp5exqxxyasq0muyl3leyxst0lle0l4l6qv9r5jj2pkz2tuljjvddxg5n2ve39xlnevesk77ze3wghzutetpzev6trdpc85jnhkec4hzz3desev5dn2ps95468v32ky7znwes9umak2pm2yhmxg9dtqhjltdyhqjt72p3842j7fen9ada7den4lzrmvfgy565l4wgyzuzgvee8w653teq9uw73jyvq5lln4vcyjxaeaxy84706tkwl7mtae7vafymyxuvymr90997aj9e9m8el7ljj9lghg304w3w4vur7rlghjnfc5ns5hv4f8x8ps3yk5jqemy4s5ye8wek93w5lrpu98vlyj6t0n0nnlqdm2ljavq4h4najl7vez2vjanupq5lhrtr2jjex7axyq39fx86axqct5c5rspalar4wnqsrd8lgkf37x0cgggrtqucd4jhr3whjv967fsdpteulp87w0s3p83kdrqsg73k2tgk5njqnaff9d5pu94z6q8vj5mg0guwq5gy343zu3pzqqj494rgsu579q2xqu4uvzyc8c5vzgp9p3yplr437ydnevutcw93sfprsymtnkgrlvd9g8ccsvzzlgvey7vphzy797qkjqr3pxdqu2vuz8zhgqe27k0lpwpxmqm6puf8cg4qjncts9knusrgk28yyqqqsukpedq6pznw5

greimela avatar Feb 02 '24 14:02 greimela

You're going to need to add test coverage of the if eve_spend branch in order to get CI to pass:

chia/cmds/wallet_funcs.py (11.1%): Missing lines 724,727-732,734

Quexington avatar Mar 04 '24 20:03 Quexington

@Quexington Can you point me to where this whole NFT royalty branch is covered? I can't see the coverage report from the CI.

I've found tests/cmds/wallet/test_wallet.py, but it doesn't cover the NFT royalty branch at all. But I can add another test there, if that is the best place.

greimela avatar Mar 05 '24 10:03 greimela

@Quexington Can you point me to where this whole NFT royalty branch is covered? I can't see the coverage report from the CI.

I've found tests/cmds/wallet/test_wallet.py, but it doesn't cover the NFT royalty branch at all. But I can add another test there, if that is the best place.

Hey @greimela sorry for the delay, I missed the notification somehow. I believe that branch is covered in tests/cmds/wallet/test_nft.py

Quexington avatar Mar 28 '24 15:03 Quexington

This PR has been flagged as stale due to no activity for over 60 days. It will not be automatically closed, but it has been given a stale-pr label and should be manually reviewed by the relevant parties.

github-actions[bot] avatar May 13 '24 11:05 github-actions[bot]

I’ll get back to this and add some more to fix the nft_get_info endpoint too

greimela avatar May 13 '24 12:05 greimela

This pull request has conflicts, please resolve those before we can evaluate the pull request.

github-actions[bot] avatar Jun 24 '24 23:06 github-actions[bot]

Closing PR - freel free to reopen and complete per discussion

emlowe avatar Jul 16 '24 15:07 emlowe