ebtc-protocol / ebtc-multisig

eBTC's EVM multisig operations.
GNU Affero General Public License v3.0
0 stars 0 forks source link

feat: add cdp ops methods in ebtc class #4

Closed petrovska-petro closed 7 months ago

petrovska-petro commented 8 months ago

Test command: brownie test --network sepolia-fork -s

Screenshot 2023-12-18 at 20 19 37
petrovska-petro commented 8 months ago

one thought i had to avoid confusions and tracking of the cdps is probably to have an cdp_address_book where pointing to those bytes is intuitive for the scripts, based on:

otherwise, i find not ux friendly to use some of the methods which requires cdp_ip as input

lmk your thoughts @sajanrajdev

sajanrajdev commented 8 months ago

one thought i had to avoid confusions and tracking of the cdps is probably to have an cdp_address_book where pointing to those bytes is intuitive for the scripts, based on:

  • cr ratio, timestamp, eth price at opening etc

otherwise, i find not ux friendly to use some of the methods which requires cdp_ip as input

lmk your thoughts @sajanrajdev

@petrovska-petro, I 100% agree. I think that putting a script together to fetch this in realtime would be handy for multisig operators but at some point we should leverage the SDK and the Graph to build a simple UI around this. I can see other users benefiting from it. Will coordinate with the Web2 team. In the meantime, I already ask the Soldity team if the MultiCDPGetter contract will be patched for the next deployment. If so, we could use this to write an interim script quite easily.

Outside of this and the final notes I dropped, everything LGTM. Scripts will need adjustment to use the final multisig decided for these operations but they are simple and accurate as intended. Tests introduce full unit coverage which should be enough here. All tests are passing.

image

petrovska-petro commented 8 months ago

if the MultiCDPGetter contract will be patched for the next deployment. If so, we could use this to write an interim script quite easily.

the approach seems straight forward doable for the cdp insight per owner, i like it. def, worth adding once latest deployments hits testnet