search

ergoplatform / ergo-appkit

Appkit: A Library for Java/Scala/Kotlin Development of Ergo Applications
MIT License
38 stars 27 forks source link

Added Node Wallet support to BlockchainDataSource, along with a simple BoxLoader #164

Open K-Singh opened 2 years ago

K-Singh commented 2 years ago

Would like a review, because implementing IUnspentBoxesLoader feels wrong due to needing an Address. Also no way to paginate boxes from the WalletAPI as far as I can tell, so had to pre-load all boxes which may be inefficient. I used to use ctx.getWallet quite a bit to get wallet boxes, so would like this functionality to still be supported some how.

MrStahlfelge commented 2 years ago

Can you fix the tests?

MrStahlfelge commented 2 years ago

We removed the wallet functionality from blockchain datasource completely: It is a functionality directly derived from restrictions of the node (it is not able to fetch unspend wallets for all addresses, but only for the own wallet), and it is not something the blockchain data is defining.

To work around this restriction without using the explorer you could introduce a NodeWalletDatasource implementing the getUnspentBoxesFor methods in a way that they actually return the node wallet's boxes.

However, I would completely dismiss this and work with the using an own loader only.

K-Singh commented 2 years ago

Can you fix the tests?

Yeah, didn't catch those before, will get a fix in tomorrow.

We removed the wallet functionality from blockchain datasource completely: It is a functionality directly derived from restrictions of the node (it is not able to fetch unspend wallets for all addresses, but only for the own wallet), and it is not something the blockchain data is defining.

To work around this restriction without using the explorer you could introduce a NodeWalletDatasource implementing the getUnspentBoxesFor methods in a way that they actually return the node wallet's boxes.

However, I would completely dismiss this and work with the using an own loader only.

Sure, I can create a NodeWalletDataSource specifically for this. I don't entirely understand what you mean by the last line though.

MrStahlfelge commented 2 years ago

Sure, I can create a NodeWalletDataSource specifically for this. I don't entirely understand what you mean by the last line though.

The following: Instead of adapting the data source, you can just use the loader to fetch the data from the node. Instead of

 allBoxes = dataSource.getUnconfirmedUnspentWalletBoxes();

you do

allBoxes = (dataSource as NodeAndExplorerDataSourceImpl).getWalletApi().getUnconfirmedUnspentWalletBoxes();

No need to change the datasource itself, just use an own loader. You can do this in your project without changing appkit.