fix: fix exceeded the prepaid gas when upgrade remote contract

[fospring]

No description provided.

[fospring]

When GAS_FOR_UPGRADE_REMOTE_DEPLOY is 15T, upgrade remote contract will throw error of Exceeded the prepaid gas. Updated GAS_FOR_UPGRADE_REMOTE_DEPLOY to 25T would throw same error, So I updated it to 30T.

This is the failed tx when GAS_FOR_UPGRADE_REMOTE_DEPLOY is 25T: https://explorer.testnet.near.org/transactions/AR5j8Dz3Ts4ZDKXr1Zd5Eq7VGeS23AmWDtpjhih2wq2m

[agileurbanite]

LGTM, I don't see this negatively impacting other DAOs

[agileurbanite]

But I believe you'll need @ilblackdragon approval given that he's the main contributor to sputnik atm.

[fospring]

@ilblackdragon can you pls review this MR?

[ilblackdragon]

I'm having trouble opening the execution plan for this transaction (https://testnet.nearblocks.io/txns/AR5j8Dz3Ts4ZDKXr1Zd5Eq7VGeS23AmWDtpjhih2wq2m#execution)

Mostly question is why this is needed, because this change may be hiding another issue. LEFTOVER parameters are to cover costs of calling promise_function_call itself and returning Promise and it should be not be taking this much gas. It shouldn't even be 15Tg in the first place.

There is also new API in 4.0 SDK via near/NEPs#264 that allows to not have this at all, at it calculates leftover gas after function already completed: near/near-sdk-rs#740

If we are updating this, let's switch to this API given sputnikdao2 is already on 4.0 sdk.

[fospring]

I'm having trouble opening the execution plan for this transaction (https://testnet.nearblocks.io/txns/AR5j8Dz3Ts4ZDKXr1Zd5Eq7VGeS23AmWDtpjhih2wq2m#execution)

Mostly question is why this is needed, because this change may be hiding another issue. LEFTOVER parameters are to cover costs of calling promise_function_call itself and returning Promise and it should be not be taking this much gas. It shouldn't even be 15Tg in the first place.

There is also new API in 4.0 SDK via near/NEPs#264 that allows to not have this at all, at it calculates leftover gas after function already completed: near/near-sdk-rs#740

If we are updating this, let's switch to this API given sputnikdao2 is already on 4.0 sdk.

[fospring]

I'm having trouble opening the execution plan for this transaction (https://testnet.nearblocks.io/txns/AR5j8Dz3Ts4ZDKXr1Zd5Eq7VGeS23AmWDtpjhih2wq2m#execution)

Mostly question is why this is needed, because this change may be hiding another issue. LEFTOVER parameters are to cover costs of calling promise_function_call itself and returning Promise and it should be not be taking this much gas. It shouldn't even be 15Tg in the first place.

There is also new API in 4.0 SDK via near/NEPs#264 that allows to not have this at all, at it calculates leftover gas after function already completed: near/near-sdk-rs#740

If we are updating this, let's switch to this API given sputnikdao2 is already on 4.0 sdk.

May be there are some trouble for FE in query execution plan:
https://testnet.nearblocks.io/txns/AR5j8Dz3Ts4ZDKXr1Zd5Eq7VGeS23AmWDtpjhih2wq2m#

[agileurbanite]

We need to migrate sputnikdao2 to the latest sdk as it's not even on 4.0 it's currently on 4.0.0-pre.4 which is not even defined as a release version on github:
https://github.com/near/near-sdk-rs/releases?expanded=true&page=2&q=4.0.0-pre.4

But strangely there are rust docs on it:
https://docs.rs/near-sdk/4.0.0-pre.4/near_sdk/index.html

Either way, I'm not sure near/NEPs#264 was even introduced in 4.0.0-pre.5 as it looks like it was pretty of the changes for 4.0.0 stable release here: https://github.com/near/near-sdk-rs/releases/tag/4.0.0

What does this mean?

Well to migrate sputnikdao2 to >= 4.0.0 a few things need to happen:

1. Migrate to the new rust SDK version and update deprecated methods
2. Remove near-sdk-sim and migrate to workspaces-rs

The above is covered in:
#198

Overall, however, my perception of the task is that it seems like a larger effort.

@ilblackdragon do you have concerns with increasing FACTORY_UPDATE_GAS_LEFTOVER as a stop gap for now and then during the follow up with updating the SDK we can re-evaluate the need for this as part of that effort.

Also @fospring to @ilblackdragon point, I do believe you need to re-evaluate your implementation as it should not take 15Tg in the first place. Can you share pseudo code of your implementation or your execution plan in detail?

[fospring]

We need to migrate sputnikdao2 to the latest sdk as it's not even on 4.0 it's currently on 4.0.0-pre.4 which is not even defined as a release version on github: https://github.com/near/near-sdk-rs/releases?expanded=true&page=2&q=4.0.0-pre.4

But strangely there are rust docs on it: https://docs.rs/near-sdk/4.0.0-pre.4/near_sdk/index.html

Either way, I'm not sure near/NEPs#264 was even introduced in 4.0.0-pre.5 as it looks like it was pretty of the changes for 4.0.0 stable release here: https://github.com/near/near-sdk-rs/releases/tag/4.0.0

What does this mean?

Well to migrate sputnikdao2 to >= 4.0.0 a few things need to happen:

1. Migrate to the new rust SDK version and update deprecated methods
2. Remove near-sdk-sim and migrate to workspaces-rs

The above is covered in: #198

Overall, however, my perception of the task is that it seems like a larger effort.

@ilblackdragon do you have concerns with increasing FACTORY_UPDATE_GAS_LEFTOVER as a stop gap for now and then during the follow up with updating the SDK we can re-evaluate the need for this as part of that effort.

Also @fospring to @ilblackdragon point, I do believe you need to re-evaluate your implementation as it should not take 15Tg in the first place. Can you share pseudo code of your implementation or your execution plan in detail?

@agileurbanite @ilblackdragon My executions flows:

1. create mock accounts:

• or1007-2.yongchun.testnet
• or1007owner-2.yongchun.testnet

2. build dao :

git clone [email protected]:near-daos/sputnik-dao-contract.git
cargo build --release -p sputnikdao2 --target wasm32-unknown-unknown

3. deploy dao contract(or1007owner-2.yongchun.testnet, owner of or1007-2.yongchun.testnet) by this transaction
4. deploy smart contract to or1007-2.yongchun.testnet with owner or1007owner-2.yongchun.testnet by this transaction
5. store-blob of smart contract, ready to upgrade smart contract of or1007-2.yongchun.testnet by this transaction, the store-blob's base58_crypto hash is Hy8JJwzpRvhnSnEoQnF1WvDU5kZFksAbiScXsDeuzLZg
6. add proposal of remote upgrade or1007-2.yongchun.testnet smart contract, and act proposal:

• add proposal transaction, the proposal id is 0
• act proposal transaction by one approver
• act proposal transaction by another approver with attached 300T gas but failed with error: ExecutionError": "Exceeded the prepaid gas.
  transaction execution plan in testnet.nearblocks
  

I think I have tested that 15T gas LEFTOVER is not enough for action of UpgradeRemote.

[fospring]

We need to migrate sputnikdao2 to the latest sdk as it's not even on 4.0 it's currently on 4.0.0-pre.4 which is not even defined as a release version on github: https://github.com/near/near-sdk-rs/releases?expanded=true&page=2&q=4.0.0-pre.4

But strangely there are rust docs on it: https://docs.rs/near-sdk/4.0.0-pre.4/near_sdk/index.html

Either way, I'm not sure near/NEPs#264 was even introduced in 4.0.0-pre.5 as it looks like it was pretty of the changes for 4.0.0 stable release here: https://github.com/near/near-sdk-rs/releases/tag/4.0.0

What does this mean?

Well to migrate sputnikdao2 to >= 4.0.0 a few things need to happen:

1. Migrate to the new rust SDK version and update deprecated methods
2. Remove near-sdk-sim and migrate to workspaces-rs

The above is covered in: #198

Overall, however, my perception of the task is that it seems like a larger effort.

@ilblackdragon do you have concerns with increasing FACTORY_UPDATE_GAS_LEFTOVER as a stop gap for now and then during the follow up with updating the SDK we can re-evaluate the need for this as part of that effort.

Also @fospring to @ilblackdragon point, I do believe you need to re-evaluate your implementation as it should not take 15Tg in the first place. Can you share pseudo code of your implementation or your execution plan in detail?

@agileurbanite @ilblackdragon, Thank you for your reminder. I have added a new transaction test that when FACTORY_UPDATE_GAS_LEFTOVER is 15Tg, it's enough, so I reverted updation of FACTORY_UPDATE_GAS_LEFTOVER by this commit e2ab7ad036c0a842fd69ba05f7bceb12cc740f84

pls check again

[fospring]

We need to migrate sputnikdao2 to the latest sdk as it's not even on 4.0 it's currently on 4.0.0-pre.4 which is not even defined as a release version on github: https://github.com/near/near-sdk-rs/releases?expanded=true&page=2&q=4.0.0-pre.4

But strangely there are rust docs on it: https://docs.rs/near-sdk/4.0.0-pre.4/near_sdk/index.html

Either way, I'm not sure near/NEPs#264 was even introduced in 4.0.0-pre.5 as it looks like it was pretty of the changes for 4.0.0 stable release here: https://github.com/near/near-sdk-rs/releases/tag/4.0.0

What does this mean?

Well to migrate sputnikdao2 to >= 4.0.0 a few things need to happen:

1. Migrate to the new rust SDK version and update deprecated methods
2. Remove near-sdk-sim and migrate to workspaces-rs

The above is covered in: #198

Overall, however, my perception of the task is that it seems like a larger effort.

@ilblackdragon do you have concerns with increasing FACTORY_UPDATE_GAS_LEFTOVER as a stop gap for now and then during the follow up with updating the SDK we can re-evaluate the need for this as part of that effort.

Also @fospring to @ilblackdragon point, I do believe you need to re-evaluate your implementation as it should not take 15Tg in the first place. Can you share pseudo code of your implementation or your execution plan in detail?

@agileurbanite @ilblackdragon can we hotfix on this issue? https://explorer.testnet.near.org/transactions/3xRbdcbdQnC1jSo2arzzQ6bJDNs7PPdrLARmvYFbRL68