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

imp: use uint64 for height everywhere #366

Merged
merged 9 commits into from
Mar 11, 2025
Merged

imp: use uint64 for height everywhere #366

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
a94190a
imp: use uint64 for height everywhere
gjermundgaraba Mar 5, 2025
15ba27c
update gas benchmark + code review fixes
gjermundgaraba Mar 6, 2025
94f8aab
Merge branch 'main' into gjermund/height-to-uint64
srdtrk Mar 6, 2025
bc6be03
add aggregated gas updates
gjermundgaraba Mar 6, 2025
c966369
Merge branch 'main' into gjermund/height-to-uint64
srdtrk Mar 7, 2025
35ef6ab
Merge branch 'main' into gjermund/height-to-uint64
gjermundgaraba Mar 7, 2025
3dc7a45
Merge branch 'main' into gjermund/height-to-uint64
gjermundgaraba Mar 8, 2025
f924073
Merge branch 'main' into gjermund/height-to-uint64
gjermundgaraba Mar 9, 2025
39590a7
Merge branch 'main' into gjermund/height-to-uint64
srdtrk Mar 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,10 +190,10 @@ The following benchmarks are for a single packet transfer without aggregation.
| **Contract** | **Method** | **Description** | **Gas (groth16)** | **Gas (plonk)** |
|:---:|:---:|:---:|:---:|:---:|
| `ICS26Router.sol` | `sendPacket` | Initiating an IBC transfer with an `ERC20`. | ~165,000 | ~165,000 |
| `ICS26Router.sol` | `recvPacket` | Receiving _back_ an `ERC20` token. | ~518,643 | ~601,729 |
| `ICS26Router.sol` | `recvPacket` | Receiving a _new_ Cosmos token for the first time. (Deploying an `ERC20` contract) | ~1,087,456 | ~1,171,586 |
| `ICS26Router.sol` | `ackPacket` | Acknowledging an ICS20 packet. | ~393,398 | ~477,221 |
| `ICS26Router.sol` | `timeoutPacket` | Timing out an ICS20 packet | ~466,007 | ~549,322 |
| `ICS26Router.sol` | `recvPacket` | Receiving _back_ an `ERC20` token. | ~518,340 | ~602,020 |
| `ICS26Router.sol` | `recvPacket` | Receiving a _new_ Cosmos token for the first time. (Deploying an `ERC20` contract) | ~1,087,699 | ~1,171,433 |
| `ICS26Router.sol` | `ackPacket` | Acknowledging an ICS20 packet. | ~392,851 | ~476,578 |
| `ICS26Router.sol` | `timeoutPacket` | Timing out an ICS20 packet | ~466,083 | ~550,208 |

### Aggregated Packet Benchmarks

Expand All @@ -202,8 +202,8 @@ Since there is no meaningful difference in gas costs between plonk and groth16 i

| **ICS26Router Method** | **Description** | **Avg Gas (25 packets)** | **Avg Gas (50 packets)** | **Calldata size (25 packets)** | **Calldata size (50 packets)** |
|:---:|:---:|:---:|:---:|:---:|:---:|
| `multicall/recvPacket` | Receiving _back_ an `ERC20` token. | ~178,272 | ~165,240 | ~51,172B | ~100,772B |
| `multicall/ackPacket` | Acknowledging an ICS20 packet. | ~92,269 | ~86,564 | ~53,572B | ~105,572B |
| `multicall/recvPacket` | Receiving _back_ an `ERC20` token. | ~178,487 | ~172,258 | ~51,172B | ~100,772B |
Copy link
Member

Choose a reason for hiding this comment

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

avg 50 packet receive increased by 8k?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, seems like I forgot to update that one last time in main :/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I got 172,275 running it on main

| `multicall/ackPacket` | Acknowledging an ICS20 packet. | ~91,722 | ~86,018 | ~53,572B | ~105,572B |

Note: These gas benchmarks are with Groth16.

Expand Down
24 changes: 12 additions & 12 deletions abi/ICS26Router.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,13 +141,13 @@
"components": [
{
"name": "revisionNumber",
"type": "uint32",
"internalType": "uint32"
"type": "uint64",
"internalType": "uint64"
},
{
"name": "revisionHeight",
"type": "uint32",
"internalType": "uint32"
"type": "uint64",
"internalType": "uint64"
}
]
}
Expand Down Expand Up @@ -901,13 +901,13 @@
"components": [
{
"name": "revisionNumber",
"type": "uint32",
"internalType": "uint32"
"type": "uint64",
"internalType": "uint64"
},
{
"name": "revisionHeight",
"type": "uint32",
"internalType": "uint32"
"type": "uint64",
"internalType": "uint64"
}
]
}
Expand Down Expand Up @@ -1203,13 +1203,13 @@
"components": [
{
"name": "revisionNumber",
"type": "uint32",
"internalType": "uint32"
"type": "uint64",
"internalType": "uint64"
},
{
"name": "revisionHeight",
"type": "uint32",
"internalType": "uint32"
"type": "uint64",
"internalType": "uint64"
}
]
}
Expand Down
28 changes: 14 additions & 14 deletions abi/SP1ICS07Tendermint.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,13 +152,13 @@
"components": [
{
"name": "revisionNumber",
"type": "uint32",
"internalType": "uint32"
"type": "uint64",
"internalType": "uint64"
},
{
"name": "revisionHeight",
"type": "uint32",
"internalType": "uint32"
"type": "uint64",
"internalType": "uint64"
}
]
},
Expand Down Expand Up @@ -204,8 +204,8 @@
"inputs": [
{
"name": "revisionHeight",
"type": "uint32",
"internalType": "uint32"
"type": "uint64",
"internalType": "uint64"
}
],
"outputs": [
Expand Down Expand Up @@ -302,13 +302,13 @@
"components": [
{
"name": "revisionNumber",
"type": "uint32",
"internalType": "uint32"
"type": "uint64",
"internalType": "uint64"
},
{
"name": "revisionHeight",
"type": "uint32",
"internalType": "uint32"
"type": "uint64",
"internalType": "uint64"
}
]
},
Expand Down Expand Up @@ -355,13 +355,13 @@
"components": [
{
"name": "revisionNumber",
"type": "uint32",
"internalType": "uint32"
"type": "uint64",
"internalType": "uint64"
},
{
"name": "revisionHeight",
"type": "uint32",
"internalType": "uint32"
"type": "uint64",
"internalType": "uint64"
}
]
},
Expand Down
42 changes: 21 additions & 21 deletions abigen/ics26router/contract.go
View file
Open in desktop

Large diffs are not rendered by default.

54 changes: 27 additions & 27 deletions abigen/sp1ics07tendermint/contract.go
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion contracts/light-clients/ISP1ICS07Tendermint.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,5 +33,5 @@ interface ISP1ICS07Tendermint {
/// @notice Returns the consensus state keccak256 hash at the given revision height.
/// @param revisionHeight The revision height.
/// @return The consensus state at the given revision height.
function getConsensusStateHash(uint32 revisionHeight) external view returns (bytes32);
function getConsensusStateHash(uint64 revisionHeight) external view returns (bytes32);
}
10 changes: 5 additions & 5 deletions contracts/light-clients/SP1ICS07Tendermint.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ contract SP1ICS07Tendermint is ISP1ICS07TendermintErrors, ISP1ICS07Tendermint, I
IICS07TendermintMsgs.ClientState public clientState;
/// @notice The mapping from height to consensus state keccak256 hashes.
/// @dev Revision number need not be keyed as it is not allowed to change.
mapping(uint32 height => bytes32 hash) private _consensusStateHashes;
mapping(uint64 height => bytes32 hash) private _consensusStateHashes;

/// @inheritdoc ISP1ICS07Tendermint
uint16 public constant ALLOWED_SP1_CLOCK_DRIFT = 30 minutes;
Expand Down Expand Up @@ -85,7 +85,7 @@ contract SP1ICS07Tendermint is ISP1ICS07TendermintErrors, ISP1ICS07Tendermint, I
}

/// @inheritdoc ISP1ICS07Tendermint
function getConsensusStateHash(uint32 revisionHeight) public view returns (bytes32) {
function getConsensusStateHash(uint64 revisionHeight) public view returns (bytes32) {
bytes32 hash = _consensusStateHashes[revisionHeight];
require(hash != 0, ConsensusStateNotFound());
return hash;
Expand Down Expand Up @@ -369,7 +369,7 @@ contract SP1ICS07Tendermint is ISP1ICS07TendermintErrors, ISP1ICS07Tendermint, I
/// @param trustedConsensusState The trusted consensus state
function _validateMembershipOutput(
bytes32 outputCommitmentRoot,
uint32 proofHeight,
uint64 proofHeight,
IICS07TendermintMsgs.ConsensusState memory trustedConsensusState
)
private
Expand Down Expand Up @@ -511,7 +511,7 @@ contract SP1ICS07Tendermint is ISP1ICS07TendermintErrors, ISP1ICS07Tendermint, I
/// @param timestamp The timestamp of the trusted consensus state.
/// @dev WARNING: Transient store is not reverted even if a message within a transaction reverts.
/// @dev WARNING: This function must be called after all proof and validation checks.
function _cacheKvPairs(uint32 proofHeight, IMembershipMsgs.KVPair[] memory kvPairs, uint256 timestamp) private {
function _cacheKvPairs(uint64 proofHeight, IMembershipMsgs.KVPair[] memory kvPairs, uint256 timestamp) private {
for (uint256 i = 0; i < kvPairs.length; i++) {
bytes32 kvPairHash = keccak256(abi.encode(proofHeight, kvPairs[i]));
kvPairHash.asUint256().tstore(timestamp);
Expand All @@ -523,7 +523,7 @@ contract SP1ICS07Tendermint is ISP1ICS07TendermintErrors, ISP1ICS07Tendermint, I
/// @param kvPair The key-value pair.
/// @return The timestamp of the cached key-value pair.
function _getCachedKvPair(
uint32 proofHeight,
uint64 proofHeight,
IMembershipMsgs.KVPair memory kvPair
)
private
Expand Down
4 changes: 2 additions & 2 deletions contracts/msgs/IICS02ClientMsgs.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ interface IICS02ClientMsgs {
/// @param revisionNumber The revision number of the counterparty chain
/// @param revisionHeight The height of the counterparty chain
struct Height {
uint32 revisionNumber;
uint32 revisionHeight;
uint64 revisionNumber;
uint64 revisionHeight;
}
}
4 changes: 2 additions & 2 deletions e2e/interchaintestv8/ibc_eureka_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -308,8 +308,8 @@ func (s *IbcEurekaTestSuite) DeployTest(ctx context.Context, proofType operator.
s.Require().Equal(uint32(testvalues.DefaultTrustPeriod), clientState.TrustingPeriod)
s.Require().Equal(uint32(stakingParams.UnbondingTime.Seconds()), clientState.UnbondingPeriod)
s.Require().False(clientState.IsFrozen)
s.Require().Equal(uint32(1), clientState.LatestHeight.RevisionNumber)
s.Require().Greater(clientState.LatestHeight.RevisionHeight, uint32(0))
s.Require().Equal(uint64(1), clientState.LatestHeight.RevisionNumber)
s.Require().Greater(clientState.LatestHeight.RevisionHeight, uint64(0))
}))

s.Require().True(s.Run("Verify ICS02 Client", func() {
Expand Down
8 changes: 4 additions & 4 deletions e2e/interchaintestv8/multichain_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -466,8 +466,8 @@ func (s *MultichainTestSuite) TestDeploy_Groth16() {
s.Require().Equal(uint32(testvalues.DefaultTrustPeriod), clientState.TrustingPeriod)
s.Require().Equal(uint32(stakingParams.UnbondingTime.Seconds()), clientState.UnbondingPeriod)
s.Require().False(clientState.IsFrozen)
s.Require().Equal(uint32(1), clientState.LatestHeight.RevisionNumber)
s.Require().Greater(clientState.LatestHeight.RevisionHeight, uint32(0))
s.Require().Equal(uint64(1), clientState.LatestHeight.RevisionNumber)
s.Require().Greater(clientState.LatestHeight.RevisionHeight, uint64(0))
}))

s.Require().True(s.Run("Verify SimdB SP1 Client", func() {
Expand All @@ -483,8 +483,8 @@ func (s *MultichainTestSuite) TestDeploy_Groth16() {
s.Require().Equal(uint32(testvalues.DefaultTrustPeriod), clientState.TrustingPeriod)
s.Require().Equal(uint32(stakingParams.UnbondingTime.Seconds()), clientState.UnbondingPeriod)
s.Require().False(clientState.IsFrozen)
s.Require().Equal(uint32(2), clientState.LatestHeight.RevisionNumber)
s.Require().Greater(clientState.LatestHeight.RevisionHeight, uint32(0))
s.Require().Equal(uint64(2), clientState.LatestHeight.RevisionNumber)
s.Require().Greater(clientState.LatestHeight.RevisionHeight, uint64(0))
}))

s.Require().True(s.Run("Verify ICS02 Client", func() {
Expand Down
12 changes: 6 additions & 6 deletions e2e/interchaintestv8/operator/operator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,8 @@ func MembershipProof(trusted_height uint64, paths string, writeFixtureName strin
}

heightType, err := abi.NewType("tuple", "IICS02ClientMsgsHeight", []abi.ArgumentMarshaling{
{Name: "revisionNumber", Type: "uint32"},
{Name: "revisionHeight", Type: "uint32"},
{Name: "revisionNumber", Type: "uint64"},
{Name: "revisionHeight", Type: "uint64"},
})
if err != nil {
return nil, nil, err
Expand All @@ -124,7 +124,7 @@ func MembershipProof(trusted_height uint64, paths string, writeFixtureName strin

height := abi.ConvertType(heightI[0], new(sp1ics07tendermint.IICS02ClientMsgsHeight)).(*sp1ics07tendermint.IICS02ClientMsgsHeight)

if height.RevisionHeight != uint32(trusted_height) {
if height.RevisionHeight != trusted_height {
return nil, nil, errors.New("heights do not match")
}

Expand Down Expand Up @@ -164,8 +164,8 @@ func UpdateClientAndMembershipProof(trusted_height, target_height uint64, paths
}

heightType, err := abi.NewType("tuple", "IICS02ClientMsgsHeight", []abi.ArgumentMarshaling{
{Name: "revisionNumber", Type: "uint32"},
{Name: "revisionHeight", Type: "uint32"},
{Name: "revisionNumber", Type: "uint64"},
{Name: "revisionHeight", Type: "uint64"},
})
if err != nil {
return nil, nil, err
Expand All @@ -183,7 +183,7 @@ func UpdateClientAndMembershipProof(trusted_height, target_height uint64, paths

height := abi.ConvertType(heightI[0], new(sp1ics07tendermint.IICS02ClientMsgsHeight)).(*sp1ics07tendermint.IICS02ClientMsgsHeight)

if height.RevisionHeight != uint32(target_height) {
if height.RevisionHeight != target_height {
return nil, nil, errors.New("heights do not match")
}

Expand Down
24 changes: 12 additions & 12 deletions e2e/interchaintestv8/sp1_ics07_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,8 +161,8 @@ func (s *SP1ICS07TendermintTestSuite) DeployTest(ctx context.Context, pt operato
s.Require().Equal(uint32(testvalues.DefaultTrustPeriod), clientState.TrustingPeriod)
s.Require().Equal(uint32(stakingParams.UnbondingTime.Seconds()), clientState.UnbondingPeriod)
s.Require().False(clientState.IsFrozen)
s.Require().Equal(uint32(1), clientState.LatestHeight.RevisionNumber)
s.Require().Greater(clientState.LatestHeight.RevisionHeight, uint32(0))
s.Require().Equal(uint64(1), clientState.LatestHeight.RevisionNumber)
s.Require().Greater(clientState.LatestHeight.RevisionHeight, uint64(0))
}))
}

Expand Down Expand Up @@ -206,7 +206,7 @@ func (s *SP1ICS07TendermintTestSuite) UpdateClientTest(ctx context.Context, pt o
s.Require().Equal(uint32(testvalues.DefaultTrustPeriod), clientState.TrustingPeriod)
s.Require().Equal(uint32(stakingParams.UnbondingTime.Seconds()), clientState.UnbondingPeriod)
s.Require().False(clientState.IsFrozen)
s.Require().Equal(uint32(1), clientState.LatestHeight.RevisionNumber)
s.Require().Equal(uint64(1), clientState.LatestHeight.RevisionNumber)
s.Require().Greater(clientState.LatestHeight.RevisionHeight, initialHeight)
}))
}
Expand Down Expand Up @@ -264,7 +264,7 @@ func (s *SP1ICS07TendermintTestSuite) MembershipTest(pt operator.SupportedProofT

memArgs := append([]string{"--trust-level", testvalues.DefaultTrustLevel.String(), "--trusting-period", strconv.Itoa(testvalues.DefaultTrustPeriod), "--base64"}, pt.ToOperatorArgs()...)
proofHeight, ucAndMemProof, err := operator.MembershipProof(
uint64(trustedHeight), operator.ToBase64KeyPaths(membershipKey), "",
trustedHeight, operator.ToBase64KeyPaths(membershipKey), "",
memArgs...,
)
s.Require().NoError(err)
Expand Down Expand Up @@ -302,7 +302,7 @@ func (s *SP1ICS07TendermintTestSuite) MembershipTest(pt operator.SupportedProofT

nonMemArgs := append([]string{"--trust-level", testvalues.DefaultTrustLevel.String(), "--trusting-period", strconv.Itoa(testvalues.DefaultTrustPeriod), "--base64"}, pt.ToOperatorArgs()...)
proofHeight, ucAndMemProof, err := operator.MembershipProof(
uint64(trustedHeight), operator.ToBase64KeyPaths(nonMembershipKey), "",
trustedHeight, operator.ToBase64KeyPaths(nonMembershipKey), "",
nonMemArgs...,
)
s.Require().NoError(err)
Expand Down Expand Up @@ -371,7 +371,7 @@ func (s *SP1ICS07TendermintTestSuite) UpdateClientAndMembershipTest(ctx context.
latestHeight, err := simd.Height(ctx)
s.Require().NoError(err)

s.Require().Greater(uint32(latestHeight), trustedHeight)
s.Require().Greater(uint64(latestHeight), trustedHeight)

var expValue []byte
s.Require().True(s.Run("Get expected value for the verify membership", func() {
Expand All @@ -388,7 +388,7 @@ func (s *SP1ICS07TendermintTestSuite) UpdateClientAndMembershipTest(ctx context.

args := append([]string{"--trust-level", testvalues.DefaultTrustLevel.String(), "--trusting-period", strconv.Itoa(testvalues.DefaultTrustPeriod), "--base64"}, pt.ToOperatorArgs()...)
proofHeight, ucAndMemProof, err := operator.UpdateClientAndMembershipProof(
uint64(trustedHeight), uint64(latestHeight),
trustedHeight, uint64(latestHeight),
operator.ToBase64KeyPaths(membershipKey, nonMembershipKey),
args...,
)
Expand All @@ -413,7 +413,7 @@ func (s *SP1ICS07TendermintTestSuite) UpdateClientAndMembershipTest(ctx context.
clientState, err = s.contract.ClientState(nil)
s.Require().NoError(err)

s.Require().Equal(uint32(1), clientState.LatestHeight.RevisionNumber)
s.Require().Equal(uint64(1), clientState.LatestHeight.RevisionNumber)
s.Require().Greater(clientState.LatestHeight.RevisionHeight, trustedHeight)
s.Require().Equal(proofHeight.RevisionHeight, clientState.LatestHeight.RevisionHeight)
s.Require().False(clientState.IsFrozen)
Expand Down Expand Up @@ -452,7 +452,7 @@ func (s *SP1ICS07TendermintTestSuite) DoubleSignMisbehaviourTest(ctx context.Con

clientState, err := s.contract.ClientState(nil)
s.Require().NoError(err)
trustedHeight := clienttypes.NewHeight(uint64(clientState.LatestHeight.RevisionNumber), uint64(clientState.LatestHeight.RevisionHeight))
trustedHeight := clienttypes.NewHeight(clientState.LatestHeight.RevisionNumber, clientState.LatestHeight.RevisionHeight)

trustedHeader.TrustedHeight = trustedHeight
trustedHeader.TrustedValidators = trustedHeader.ValidatorSet
Expand Down Expand Up @@ -558,7 +558,7 @@ func (s *SP1ICS07TendermintTestSuite) BreakingTimeMonotonicityMisbehaviourTest(c

clientState, err := s.contract.ClientState(nil)
s.Require().NoError(err)
trustedHeight := clienttypes.NewHeight(uint64(clientState.LatestHeight.RevisionNumber), uint64(clientState.LatestHeight.RevisionHeight))
trustedHeight := clienttypes.NewHeight(clientState.LatestHeight.RevisionNumber, clientState.LatestHeight.RevisionHeight)

trustedHeader.TrustedHeight = trustedHeight
trustedHeader.TrustedValidators = trustedHeader.ValidatorSet
Expand Down Expand Up @@ -724,7 +724,7 @@ func (s *SP1ICS07TendermintTestSuite) UpdateClient(ctx context.Context) clientty
}))

return clienttypes.Height{
RevisionNumber: uint64(latestHeight.RevisionNumber),
RevisionHeight: uint64(latestHeight.RevisionHeight),
RevisionNumber: latestHeight.RevisionNumber,
RevisionHeight: latestHeight.RevisionHeight,
}
}
3 changes: 1 addition & 2 deletions packages/relayer-lib/src/tx_builder/cosmos_to_cosmos.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,7 @@ impl TxBuilderService<CosmosSdk, CosmosSdk> for TxBuilder {
client_state
.latest_height
.ok_or_else(|| anyhow::anyhow!("No latest height found"))?
.revision_height
.try_into()?,
.revision_height,
))
.await?;
let proposed_header = target_light_block.into_header(&trusted_light_block);
Expand Down
4 changes: 2 additions & 2 deletions packages/relayer-lib/src/tx_builder/cosmos_to_eth.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,11 +96,11 @@ where
.as_secs();

let latest_light_block = self.tm_client.get_light_block(None).await?;
let revision_height = u32::try_from(latest_light_block.height().value())?;
let revision_height = latest_light_block.height().value();
let chain_id =
ChainId::from_str(latest_light_block.signed_header.header.chain_id.as_str())?;
let latest_height = Height {
revisionNumber: chain_id.revision_number().try_into()?,
revisionNumber: chain_id.revision_number(),
revisionHeight: revision_height,
};

Expand Down
Loading
Loading