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

refactor: use cosmos sdk Int type for balances/token amounts #679

Merged
merged 3 commits into from
Aug 2, 2023
Merged

refactor: use cosmos sdk Int type for balances/token amounts #679

merged 3 commits into from
Aug 2, 2023

Conversation

jtieri
Copy link
Member

@jtieri jtieri commented Jul 29, 2023

Previously we were using int64 to represent token amounts for things like transfers and balance checks. This becomes problematic when dealing with the EVM or chains like Penumbra where we may need to handle int128 or int256.

Since Go does not have native support for integer values larger than 64-bits we would need to use the stdlib's big package. The Cosmos SDK's math package contains its own Int type that is a wrapper around Go's big.Int and offers useful helper methods and enforces a 257 bit range by checking overflow, underflow and division by zero.

@jtieri jtieri marked this pull request as ready for review July 29, 2023 06:44
@jtieri jtieri requested a review from a team as a code owner July 29, 2023 06:44
@jtieri jtieri requested a review from misko9 July 29, 2023 06:44
@akc2267 akc2267 mentioned this pull request Jul 31, 2023
Closed
36 tasks
boojamya
boojamya approved these changes Aug 1, 2023
View reviewed changes
Copy link
Contributor

@boojamya boojamya left a comment

Choose a reason for hiding this comment

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

Nice! Looks good.
A few nits on the zero int, but that's it!

examples/ibc/packet_forward_test.go Outdated Show resolved Hide resolved
conformance/test.go Outdated Show resolved Hide resolved
conformance/test.go Outdated Show resolved Hide resolved
conformance/test.go Outdated Show resolved Hide resolved
conformance/test.go Outdated Show resolved Hide resolved
@boojamya
Copy link
Contributor

boojamya commented Aug 1, 2023

Also, thinking we should probably backport this.
... for consistency and for ease of future backports.

@boojamya boojamya added the BACKPORT backport into all maintained branches label Aug 1, 2023
@jtieri
Copy link
Member Author

jtieri commented Aug 1, 2023

Also, thinking we should probably backport this. ... for consistency and for ease of future backports.

I agree, we will definitely want to port this across the supported versions that we maintain. This is going to break things for anyone using interchaintest when upgrading though, since we don't cut releases here I'm wondering what would be the most effective way of communicating this breaking change to everyone downstream.

@boojamya boojamya added the breaking includes breaking changes label Aug 1, 2023
@boojamya
Copy link
Contributor

boojamya commented Aug 1, 2023

That's a good call out.
The answer is we don't have any sort of system for that in place atm.

FWIW, I made breaking label.
My only idea is to make a changelog. But I'm debating if that is necessary.

@jtieri jtieri merged commit 8e02aef into main Aug 2, 2023
6 checks passed
@jtieri jtieri deleted the justin/refactor-bigint branch August 2, 2023 15:33
mergify bot pushed a commit that referenced this pull request Aug 2, 2023
@jtieri @mergify
refactor: use cosmos sdk Int type for balances/token amounts (#679)
5e83413 
* refactor: use csdk Int type for balances/token amounts

* chore: use ZeroInt func

(cherry picked from commit 8e02aef)

# Conflicts:
#	chain/cosmos/cosmos_chain.go
#	chain/polkadot/parachain_node.go
#	chain/polkadot/polkadot_chain.go
#	chain/polkadot/query.go
#	chain/polkadot/relay_chain_node.go
#	chain/polkadot/tx.go
#	conformance/flush.go
#	conformance/test.go
#	examples/cosmos/light_client_test.go
#	examples/hyperspace/hyperspace_test.go
#	examples/ibc/interchain_accounts_test.go
#	examples/ibc/learn_ibc_test.go
#	examples/ibc/packet_forward_test.go
#	examples/ibc/wasm/wasm_ibc_test.go
#	examples/ibc/wasm/wasm_icq_test.go
#	examples/polkadot/polkadot_chain_test.go
#	examples/polkadot/push_wasm_client_code_test.go
#	go.mod
#	ibc/types.go
#	interchain_test.go
#	internal/blockdb/messages_view_test.go
#	test_user.go
mergify bot pushed a commit that referenced this pull request Aug 2, 2023
@jtieri @mergify
refactor: use cosmos sdk Int type for balances/token amounts (#679)
ebe3992 
* refactor: use csdk Int type for balances/token amounts

* chore: use ZeroInt func

(cherry picked from commit 8e02aef)

# Conflicts:
#	chain/cosmos/cosmos_chain.go
#	chain/polkadot/parachain_node.go
#	chain/polkadot/polkadot_chain.go
#	chain/polkadot/query.go
#	chain/polkadot/relay_chain_node.go
#	chain/polkadot/tx.go
#	conformance/flush.go
#	conformance/test.go
#	examples/cosmos/light_client_test.go
#	examples/hyperspace/hyperspace_test.go
#	examples/ibc/interchain_accounts_test.go
#	examples/ibc/learn_ibc_test.go
#	examples/ibc/packet_forward_test.go
#	examples/ibc/wasm/wasm_ibc_test.go
#	examples/ibc/wasm/wasm_icq_test.go
#	examples/polkadot/polkadot_chain_test.go
#	examples/polkadot/push_wasm_client_code_test.go
#	go.mod
#	ibc/types.go
#	interchain_test.go
#	internal/blockdb/messages_view_test.go
#	test_user.go
mergify bot pushed a commit that referenced this pull request Aug 2, 2023
@jtieri @mergify
refactor: use cosmos sdk Int type for balances/token amounts (#679)
f36f41b 
* refactor: use csdk Int type for balances/token amounts

* chore: use ZeroInt func

(cherry picked from commit 8e02aef)

# Conflicts:
#	chain/polkadot/query.go
#	conformance/flush.go
#	conformance/test.go
#	examples/cosmos/light_client_test.go
#	examples/hyperspace/hyperspace_test.go
#	examples/ibc/interchain_accounts_test.go
#	examples/ibc/learn_ibc_test.go
#	examples/ibc/packet_forward_test.go
#	examples/ibc/wasm/wasm_ibc_test.go
#	examples/ibc/wasm/wasm_icq_test.go
#	examples/polkadot/polkadot_chain_test.go
#	examples/polkadot/push_wasm_client_code_test.go
#	go.mod
#	ibc/types.go
#	interchain_test.go
#	internal/blockdb/messages_view_test.go
#	test_user.go
This was referenced Aug 2, 2023
Closed
Closed
mergify bot pushed a commit that referenced this pull request Aug 2, 2023
@jtieri @mergify
refactor: use cosmos sdk Int type for balances/token amounts (#679)
59a2d31 
* refactor: use csdk Int type for balances/token amounts

* chore: use ZeroInt func

(cherry picked from commit 8e02aef)

# Conflicts:
#	chain/polkadot/query.go
#	conformance/flush.go
#	conformance/test.go
#	examples/cosmos/light_client_test.go
#	examples/hyperspace/hyperspace_test.go
#	examples/ibc/interchain_accounts_test.go
#	examples/ibc/learn_ibc_test.go
#	examples/ibc/packet_forward_test.go
#	examples/ibc/wasm/wasm_ibc_test.go
#	examples/ibc/wasm/wasm_icq_test.go
#	examples/polkadot/polkadot_chain_test.go
#	examples/polkadot/push_wasm_client_code_test.go
#	go.mod
#	ibc/types.go
#	interchain_test.go
#	internal/blockdb/messages_view_test.go
#	test_user.go
This was referenced Aug 2, 2023
Closed
Closed
@boojamya boojamya mentioned this pull request Aug 17, 2023
Merged
@boojamya boojamya mentioned this pull request Sep 6, 2023
Merged
@jtieri jtieri mentioned this pull request Nov 1, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@boojamya boojamya boojamya approved these changes

@misko9 misko9 Awaiting requested review from misko9 misko9 is a code owner automatically assigned from strangelove-ventures/interchaintest-maintainers

Assignees
No one assigned
Labels
BACKPORT backport into all maintained branches breaking includes breaking changes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jtieri @boojamya