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.

Sign up for GitHub

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

Jump to bottom

feat: reject tx if total blob size too large (backport #2202) #2268

Merged
merged 6 commits into from
Aug 15, 2023
Merged

feat: reject tx if total blob size too large (backport #2202) #2268

merged 6 commits into from
Aug 15, 2023

Conversation

mergify[bot]
Copy link
Contributor

@mergify mergify bot commented Aug 14, 2023 •
edited by rootulp

This PR backports the following commits from main to v1.x:

  1. 5b8ec41
  2. 78e5327
  3. b6f3968

To bring v1.x closer to main.

@rootulp @mergify
feat: reject tx if total blob size too large (#2202)
dbdcedd 
Closes #2156 by
implementing **Option C**

(cherry picked from commit b6f3968)

# Conflicts:
#	app/ante.go
#	app/test/check_tx_test.go
@mergify mergify bot added the conflicts label Aug 14, 2023
@MSevey MSevey requested a review from a team August 14, 2023 19:00
@rootulp rootulp self-assigned this Aug 14, 2023
rootulp and others added 4 commits August 14, 2023 15:46
@rootulp
Revert "feat: reject tx if total blob size too large (#2202)"
7edcf3d 
This reverts commit dbdcedd.
@cmwaters @rootulp
feat: migrate MsgSubmitProposal validity check to app as a ante handl…
ee143a3 
…er (#2168)

This takes the PR
https://github.com/celestiaorg/cosmos-sdk/pull/315/files which targeted
the SDK and migrates it across to `celestia-app`. This brings us one
step closer to unforking us from the SDK

The only foreseeable change from this migration is that if a user is to
locally run `ValidateBasic` it might show as being valid even although
it gets rejected in `CheckTx` or `DeliverTx`.
@rootulp
test: blob sizes that are too large (#2187)
d78095a 
Part of #2156
@rootulp
feat: reject tx if total blob size too large (#2202)
15f0b89 
Closes #2156 by
implementing **Option C**
@rootulp rootulp added this to the Mainnet milestone Aug 14, 2023
@rootulp rootulp enabled auto-merge August 14, 2023 20:03
@evan-forbes
Copy link
Member

not blocking as I don't care that much, but is there a way to not merge the first two commits?

@rootulp
Copy link
Collaborator

rootulp commented Aug 14, 2023

not blocking as I don't care that much, but is there a way to not merge the first two commits?

I can prob force-push to this branch and drop those two commits but that feels more prone to error than including them. Alternatively, I can open 3 separate backport PRs with each commit.

cmwaters
cmwaters approved these changes Aug 15, 2023
View reviewed changes
Copy link
Contributor

@cmwaters cmwaters left a comment •
edited

Choose a reason for hiding this comment

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

There's probably only two commits here of substantive difference:

But I'm easy about commit history so yolo

@rootulp rootulp merged commit 906ef24 into v1.x Aug 15, 2023
21 checks passed
@rootulp rootulp deleted the mergify/bp/v1.x/pr-2202 branch August 15, 2023 10:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cmwaters cmwaters cmwaters approved these changes

@evan-forbes evan-forbes evan-forbes approved these changes

@rootulp rootulp Awaiting requested review from rootulp

Assignees

@rootulp rootulp

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@evan-forbes @rootulp @cmwaters