Skip to content

Conversation

zavgorodnii
Copy link
Contributor

@zavgorodnii zavgorodnii commented Jun 6, 2024

This PR adds the neutron-flashloans contract that uses another address as the source of funds (in our case, the DAO core address). You can find a detailed description of the contract logic and implementation in README.md, or in the docs.

Note: the neutron-flashloans-user contract that is only used in the integration tests is added to the neutron-dev-contracts repo: neutron-org/neutron-dev-contracts#57.

@zavgorodnii zavgorodnii closed this Jun 7, 2024
@zavgorodnii zavgorodnii reopened this Jun 7, 2024
@zavgorodnii zavgorodnii marked this pull request as ready for review June 7, 2024 14:07
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It also adds the neutron-flashloans-user contract that is only used in the integration tests.

It should be in our dev-contracts repo then.

requested_amount: Vec<Coin>,
) -> Result<Vec<Coin>, ContractError> {
// Prepare the query
let all_balances_query = BankQuery::AllBalances {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is safe.

I can spam the source address with an infinite amount of TF tokens, so after the certain number of created tokens your AllBalances query will return a gas error, thus users can't get new flashloans

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you suggest having an allow list of tokens that we are ready to loan?

Copy link
Collaborator

@pr0n00gler pr0n00gler Jun 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either that, or do a separate BankQuery::Balance { address: String, denom: String } query for each denom.
That way a flashloan can be broken only if a user himself provides a lot of tokens to loan. But it's kinda his problem, not contract's

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, I like the separate query approach more


// Filter all balances leaving only the balances of the requested coins
let mut pre_loan_balances: Vec<Coin> = vec![];
for requested_coin in requested_amount {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or i can get a gas error here.

TLDR infinite loops are not good

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see your point, spamming might be an issue; will fix.

expected_balances: Vec<Coin>,
) -> Result<(), ContractError> {
// Prepare the query
let all_balances_query = BankQuery::AllBalances {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same about a potentially infinite loop

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Acknowledged, will fix!

zavgorodnii and others added 3 commits June 18, 2024 12:00
Co-authored-by: Mike Mozhaev <programmer10110@gmail.com>
Co-authored-by: Mike Mozhaev <programmer10110@gmail.com>
Co-authored-by: Mike Mozhaev <programmer10110@gmail.com>
let maybe_source_coin = all_balances_response
.amount
.iter()
.find(|x| x.denom == requested_coin.denom && requested_coin.amount.le(&x.amount));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not important but seems kinda inefficient, since if there is not enough of denom it will still check all the other coins (even thought it won't find it?)

Copy link
Contributor

@sotnikov-s sotnikov-s left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please don't forget to move the flashloan user contract to the dev contracts repo

zavgorodnii and others added 10 commits June 26, 2024 14:39
Co-authored-by: sotnikov-s <34917380+sotnikov-s@users.noreply.github.com>
Co-authored-by: sotnikov-s <34917380+sotnikov-s@users.noreply.github.com>
Co-authored-by: sotnikov-s <34917380+sotnikov-s@users.noreply.github.com>
Co-authored-by: sotnikov-s <34917380+sotnikov-s@users.noreply.github.com>
Co-authored-by: sotnikov-s <34917380+sotnikov-s@users.noreply.github.com>
Co-authored-by: Dmitry Kolupaev <dmitry.klpv@gmail.com>
Copy link
Contributor

@sotnikov-s sotnikov-s left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all but this tiny issue LGTM

Comment on lines +196 to +200
fn get_pre_loan_balances(
deps: &DepsMut,
source: Addr,
requested_amount: Vec<Coin>,
) -> Result<Vec<Coin>, ContractError> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please update comments in the func body. some of them are outdated

Andrei Zavgorodnii added 4 commits January 25, 2025 15:49
# Conflicts:
#	Cargo.lock
#	contracts/dao/neutron-chain-manager/schema/neutron-chain-manager.json
#	contracts/dao/neutron-chain-manager/schema/raw/execute.json
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.

4 participants