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

chore: upgrade to cosmos-sdk v1.16.1-sdk-v0.46.13 #2115

Merged
merged 5 commits into from
Jul 24, 2023

Conversation

rootulp
Copy link
Collaborator

@rootulp rootulp commented Jul 17, 2023

Cherry-picks a few commits to the v1.x release branch so that we can cut celestia-app v1.0.0-rc10. When we merge this, we should probably NOT squash so that the individual commit messages appear in the release notes.

Testing

I started a full consensus node on mocha-3 using a celestia-appd binary based on this PR. It is synced to the head (112,304 at the time of writing) with no issues.

Note: mocha-3 is on v1.0.0-rc9 according to docs.

@rootulp rootulp added this to the Mainnet milestone Jul 17, 2023
@rootulp rootulp self-assigned this Jul 17, 2023
@rootulp
Copy link
Collaborator Author

rootulp commented Jul 17, 2023

I think the docker-build workflow is failing b/c the v1.x release branch still uses

uses: celestiaorg/.github/.github/workflows/reusable_dockerfile_pipeline.yml@v0.2.0 # yamllint disable-line rule:line-length

hello team!

This PR uses the latest version of the common pipeline, this one:
[v0.2.2](https://github.com/celestiaorg/.github/releases/tag/v0.2.2)

Please, merge after the approval, I cannot do it from my side :)

Thanks in advance! 🚀

Jose Ramon Mañes

Signed-off-by: Jose Ramon Mañes <jose@celestia.org>
@codecov-commenter
Copy link

Codecov Report

Merging #2115 (3637b81) into v1.x (ebfc416) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             v1.x    #2115   +/-   ##
=======================================
  Coverage   21.57%   21.57%           
=======================================
  Files         126      126           
  Lines       14271    14271           
=======================================
  Hits         3079     3079           
  Misses      10895    10895           
  Partials      297      297           

SurajAnand88 and others added 3 commits July 20, 2023 10:13
Closes celestiaorg#2040

---------

Co-authored-by: Rootul P <rootulp@gmail.com>
Closes celestiaorg#2129

celestiaorg#2121 didn't actually
fix the issue because the expression

```go
NanosecondsPerYear = int64(NanosecondsPerSecond * SecondsPerYear)
```
was multiplying two `int`s together prior to the cast to `int64`. Since
the product of those two `int`s overflows an `int32`, the value would
overflow prior to the cast.

The fix proposed in this PR converts SecondsPerYear to an `int64`. Now
the expression multiplies an `int` and `int64` so the product is an
`int64` which won't overflow.

## Testing

I added this to the Makefile

```go
## build-arm: Build the celestia-appd binary into the ./build directory. Target the linux OS and ARM (32-bit) architecture.
build-arm: mod
	@cd ./cmd/celestia-appd
	@mkdir -p build/
	@Goos=linux GOARCH=arm go build $(BUILD_FLAGS) -o build/ ./cmd/celestia-appd
	@go mod tidy -compat=1.20
.PHONY: build-arm
```

Before

```shell
$ make build-arm
--> Updating go.mod
# github.com/celestiaorg/celestia-app/x/mint/types
x/mint/types/constants.go:15:29: NanosecondsPerSecond * SecondsPerYear (constant 31556952000000000 of type int) overflows int
make: *** [build-arm] Error 1
```

After

```shell
$ make build-arm
--> Updating go.mod
```
@rootulp rootulp marked this pull request as ready for review July 20, 2023 14:45
@evan-forbes evan-forbes merged commit 3e50374 into celestiaorg:v1.x Jul 24, 2023
18 checks passed
@rootulp rootulp deleted the rp/prep-v1.0.0-rc10 branch July 24, 2023 13:08
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.

None yet

6 participants