NEAR EVM changes to Balancer Core

[mikedotexe]

This PR includes all commits from:

• https://github.com/ilblackdragon/balancer-core/tree/near-evm
• https://github.com/mikedotexe/balancer-core/tree/near-evm

The intention is for this branch to merged into https://github.com/balancer-labs/balancer-core giving instructions on how to run tests and migrate to the NEAR EVM.

[artob]

Needs a little more work still, methinks. Also, consider committing package-lock.json, as per our findings in near/nearcore#3665 (comment).

[artob]

@mikedotexe For our own branch, we should also disable (comment out) the exitswapExternAmountOut test case, so that this will keep working despite dependency tree matters: that is, whether installing dependencies from scratch using npm install, versus using your known-good package-lock.json. I will open an upstream bug report about that.

[ailisp]

Needs a little more work still, methinks. Also, consider committing package-lock.json, as per our findings in near/nearcore#3665 (comment).

Just saw there's a yarn.lock, is that one reproduce passing tests?

[ilblackdragon]

Yep, I understand. I was trying to solve this before as well and didn't get to an elegant solution :( I think mine was that you need to run truffle test 2 times Script indeed should be fine, it just seems wrong to have it in the code repo. Maybe add it to near-cli? And add a check that verifies there are test accounts present and if not - prints instructions to install near-cli and running the command to create test accounts.

…

On Tue, Dec 8, 2020 at 3:23 PM Mike Purvis ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In create-near-test-accounts.js <#1 (comment)>: > @@ -0,0 +1,29 @@ +const path = require('path'); +const { readKeyFile } = require('near-web3-provider/node_modules/near-api-js/lib/key_stores/unencrypted_file_system_keystore') + +async function setupNEARAccounts() { So the reason why I landed on this ended up being a number of issues I ran into that felt like they exceeded a reasonable timebox. I think the there are a couple of issues. We're trying to setup the accounts *while* truffle config is being loaded. Ultimately, the truffle config ends in module.exports = {… I tried adding async in a number of places, but that wasn't enough. Another issue is that constructors in JavaScript cannot be async. I tried the suggestions here: https://stackoverflow.com/a/43433773/711863 but did not get this working. It would not reliably wait for all the accounts to be created. So I think the constructor + the config file don't seem to be built this way. Then I thought about how it works with ganache-cli. Typically, ganache is started, then accounts are created, and then the tests are run with the truffle config. So I feel like Truffle is already rather separated from the account creation task. I got to a point where I felt as though I was going to have to understand what Truffle wants and how it works and then said, "okay, I've spent enough time on this, just use a script." — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub <#1 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABK27UNJ7ALPFJF4PW4D7LST2YQNANCNFSM4UM6DAPA> .

-- Best regards, Illia Polosukhin

[artob]

Maybe add it to near-cli? And add a check that verifies there are test accounts present and if not - prints instructions to install near-cli and running the command to create test accounts.

Yes, this would be good. We need the test account creation in any number of situations, it'd be preferable it'd be available in easy, productized form.

[mikedotexe]

I'm going to turn this into Draft until we merge aurora-is-near/near-web3-provider#64 as that will be necessary for how we interact with the NearProvider now

[artob]

@mikedotexe For our own branch, we should also disable (comment out) the exitswapExternAmountOut test case, so that this will keep working despite dependency tree matters: that is, whether installing dependencies from scratch using npm install, versus using your known-good package-lock.json. I will open an upstream bug report about that.

I feel odd about skipping a test and hoping our PR gets merged upstream. If I were a core dev on Balancer, I would not like seeing that. Do you think we can add a Troubleshooting section to the README about this bug instead? Let's continue discussion on this, and in the meantime I see there are three tests that have exitswapExternAmountOut and I'm not sure if all or one of them would be skipped, so would need that detail from someone whose computer fails.

That's why I said "for our own branch" :) I wouldn't submit that bit upstream as part of this PR. Which reminds me, I still need to open that bug report with upstream.

[artob]

@mikedotexe I think the main thing left to do is uncluttering the top level. If I were upstream, I wouldn't merge a PR that added vendor-specific files to the top level.

[mikedotexe]

I actually think this is ready for review, however I ran into issues creating test accounts on betanet. I will paste the details I put in Discord here.
I'm trying to run Balancer tests on betanet (clean up the Balancer README from all the local development, and will link them to a page on docs for that instead) and something seems to be wrong with estimation.
env NEAR_ENV=betanet near evm-dev-init josh.betanet
This should basically say, "hey NEAR CLI, i'm using betanet, create the default (5) number of test subaccounts on josh.betanet, which should be .josh.betanet"
I get:

NotEnoughBalance [Error]: Sender josh.betanet does not have enough balance 99938411780848674000000000 for operation costing 8600000190240333500000000000
So it thinks I need 8,600 Ⓝ to do the account creations. The account has 99 Ⓝ.
This NEAR CLI command is about as easy as it gets:
https://github.com/near/near-cli/blob/master/commands/evm-dev-init.js#L22-L26

async function scheduleEVMDevInit(options) {
    const near = await connect(options);
    const account = await near.account(options.accountId);
    await utils.createTestAccounts(account, options.numAccounts);
}

Which then calls createTestAccounts located here:
https://github.com/near/near-web3-provider/blob/master/src/utils.js#L388
I'm having a hard time understanding why betanet thinks this is going to be so expensive. That seems to be a nearcore thing if I'm not mistaken, but before I open an issue wanted to check in with others.

[artob]

Nice, LGTM 👍 A couple of grammar nits, can be addressed later.

[artob]

I'm having a hard time understanding why betanet thinks this is going to be so expensive. That seems to be a nearcore thing if I'm not mistaken, but before I open an issue wanted to check in with others.

I don't know the answer, but this betanet fee issue isn't going to be due to these changes, so let's not let that hold us up.

This final pull is a lot cleaner now than the original pull. Good job, @mikedotexe 👏