Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: ictx + transfer with limited sudo call #NTRN-85 #304

Merged
merged 22 commits into from
Sep 20, 2023

Conversation

swelf19
Copy link
Contributor

@swelf19 swelf19 commented Aug 18, 2023

In order to prevent malicious contract to overuse ibc sudo call, and run heavy computing at relayer's expense, we run sudo call with certain amount of gas(dao controlled) - SudoCallLimit
We run sudo call with cacheCtx with limited amount of gas
_, err = im.ContractManagerKeeper.SudoTimeout(cacheCtx, senderAddress, packet)
If the contract tries to consume more gas than allowed with SudoCallLimit, then panic is initiated by wasmvm
We converts the out of gas panic(and only this), and process it later as general error returned from the contract, i.e. suppress the error, save failure to the storage, pay fees to relayer.
In case of any other kind of panic, we treat it as relayers fault, do not recover it. Transaction aborted.

test run: https://github.com/neutron-org/neutron-tests/actions/runs/5939595766

@swelf19 swelf19 force-pushed the feat/icatx-rework branch from 509e332 to 0fb54fc Compare August 18, 2023 14:26
@swelf19 swelf19 changed the title feat: ictx + transfer with limited sudo call feat: ictx + transfer with limited sudo call #NTRN-85 Aug 22, 2023
@pr0n00gler pr0n00gler marked this pull request as ready for review August 22, 2023 13:30
@swelf19
Copy link
Contributor Author

swelf19 commented Aug 29, 2023

x/transfer/ibc_handlers.go Outdated Show resolved Hide resolved
# Conflicts:
#	x/interchaintxs/keeper/ibc_handlers.go
#	x/interchaintxs/keeper/ibc_handlers_test.go
#	x/transfer/ibc_handlers.go
#	x/transfer/ibc_handlers_test.go
Copy link
Contributor

@NeverHappened NeverHappened left a comment

Choose a reason for hiding this comment

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

All looks great! Few minor changes requested + need to resolve conflicts

network/init-neutrond.sh Outdated Show resolved Hide resolved
app/upgrades/sdk47/upgrades.go Outdated Show resolved Hide resolved
neutronutils/utils.go Outdated Show resolved Hide resolved
x/contractmanager/types/params.go Outdated Show resolved Hide resolved
x/transfer/ibc_handlers_test.go Outdated Show resolved Hide resolved
swelf19 and others added 2 commits August 31, 2023 19:04
Co-authored-by: Dmitry Kolupaev <dmitry.klpv@gmail.com>
@swelf19
Copy link
Contributor Author

swelf19 commented Aug 31, 2023

@swelf19
Copy link
Contributor Author

swelf19 commented Sep 1, 2023

Copy link
Contributor

@NeverHappened NeverHappened left a comment

Choose a reason for hiding this comment

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

LGTM!

app/upgrades/sdk47/upgrades.go Show resolved Hide resolved
app/upgrades/sdk47/upgrades.go Outdated Show resolved Hide resolved
neutronutils/errors/errors.go Outdated Show resolved Hide resolved
neutronutils/utils.go Outdated Show resolved Hide resolved
x/transfer/ibc_handlers.go Outdated Show resolved Hide resolved
x/transfer/ibc_handlers.go Outdated Show resolved Hide resolved
neutronutils/utils.go Outdated Show resolved Hide resolved
@sotnikov-s
Copy link
Contributor

also please:

  1. respect the linter errors re files formatting;
  2. create a PR to the neutron-docs repo because these changes are definitely one of those we don't want to forget about to reflect there.

@swelf19
Copy link
Contributor Author

swelf19 commented Sep 8, 2023

also please:

  1. respect the linter errors re files formatting;
  2. create a PR to the neutron-docs repo because these changes are definitely one of those we don't want to forget about to reflect there.
  1. I want to fix the linter errors after merge all major PRs into update-sdk47 branch, cause now linter create too much noise with "deprecated" errors, which should be fixed in corresponding PRs
  2. will do

@swelf19
Copy link
Contributor Author

swelf19 commented Sep 19, 2023

@swelf19
Copy link
Contributor Author

swelf19 commented Sep 19, 2023

@pr0n00gler pr0n00gler merged commit 7cd4038 into update-sdk47 Sep 20, 2023
6 of 9 checks passed
@pr0n00gler pr0n00gler deleted the feat/icatx-rework branch May 9, 2024 18:13
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