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

Refactor: Convert iterators to array getters #596

Merged
merged 54 commits into from
Dec 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
54 commits
Select commit Hold shift + click to select a range
5e1a187
WIP convert iterators to array getters.
jtremback Dec 14, 2022
9c7bf44
WIP - compiles, fixing tests
jtremback Dec 14, 2022
c19cb8d
Unit and e2e tests work
jtremback Dec 15, 2022
c1e693e
add notes about stopping iteration
jtremback Dec 15, 2022
789997c
WIP - rename and add some notes
jtremback Dec 15, 2022
43489eb
Add types to proto
jtremback Dec 15, 2022
ce77f0a
delete unused code
jtremback Dec 15, 2022
f00ac6d
resolve naming conflict
jtremback Dec 16, 2022
a82e5ee
implement another type as proto
jtremback Dec 16, 2022
b29f08e
fixing more stuff
jtremback Dec 16, 2022
b6cb90a
delete TODOJEHAN.md
jtremback Dec 16, 2022
378f67d
adds many of Marius's iteration order comments from 599, and does som…
jtremback Dec 16, 2022
2663554
fix nil pointer deref
jtremback Dec 19, 2022
7ca833e
call GetAllConsumerChains once
mpoke Dec 19, 2022
74aaebc
expand TestGetAllChannelToChains
mpoke Dec 19, 2022
f8613c7
expand TestGetAllUnbondingOps
mpoke Dec 19, 2022
40ffea5
GetAllUnbondingOpIndexes; cleanup proto files
mpoke Dec 19, 2022
643ae1c
fix GetAllValsetUpdateBlockHeights and UTs
mpoke Dec 19, 2022
b9706cf
remove GetAllSlashAck
mpoke Dec 19, 2022
02eb634
add tests for GetFirstVscSendTimestamp
mpoke Dec 19, 2022
c67eb74
key assignment iterators
mpoke Dec 19, 2022
58d5c28
reviewed proposals
mpoke Dec 19, 2022
f876368
add TestGetAllValsetUpdateBlockHeights
mpoke Dec 19, 2022
63707e1
add TestGetAllOutstandingDowntimes
mpoke Dec 19, 2022
8d4c4da
add GetElapsedPacketMaturityTimes
mpoke Dec 19, 2022
28ad814
fix merge conflicts
mpoke Dec 19, 2022
d9f4358
fix linter
mpoke Dec 19, 2022
b14ff29
fix linter
mpoke Dec 19, 2022
87ed959
prevent implicit memory aliasing
mpoke Dec 19, 2022
4259d30
add UTC to TestPacketMaturityTime
mpoke Dec 19, 2022
e1fa7c7
fix TestPacketMaturityTime
mpoke Dec 19, 2022
6a5136e
avoid local variable name shadowing
mpoke Dec 20, 2022
a5fee38
fix merge conflict
mpoke Dec 20, 2022
9a04275
Merge branch 'main' into iterators-to-arrays
mpoke Dec 20, 2022
a2c93d2
Update x/ccv/consumer/keeper/keeper.go
mpoke Dec 21, 2022
3b80893
replace cases with packets in TestPacketMaturityTime
mpoke Dec 21, 2022
3e3f10b
add expected order to TestPacketMaturityTime
mpoke Dec 21, 2022
b4112f7
add expected order to TestGetAllHeightToValsetUpdateIDs
mpoke Dec 21, 2022
4177544
add expected order to TestGetAllOutstandingDowntimes
mpoke Dec 21, 2022
60732d5
add TestGetAllCCValidator
mpoke Dec 21, 2022
fe16a62
add expected order to TestGetAllConsumerChains
mpoke Dec 21, 2022
5229c7e
add expected order to TestGetAllChannelToChains
mpoke Dec 21, 2022
649d105
add expected order to TestGetAllUnbondingOps
mpoke Dec 21, 2022
56f957e
add expected order to TestGetAllUnbondingOpIndexes
mpoke Dec 21, 2022
9a7b31b
add expected order to TestGetAllValsetUpdateBlockHeights
mpoke Dec 21, 2022
69b050b
add expected order to TestInitTimeoutTimestamp
mpoke Dec 21, 2022
2aca355
add expected order to TestVscSendTimestamp
mpoke Dec 21, 2022
4fc4736
add expected order to TestGetAllValidatorConsumerPubKey
mpoke Dec 21, 2022
ec5645c
add expected order to TestGetAllValidatorsByConsumerAddr
mpoke Dec 21, 2022
a2ec7bc
add expected order to TestGetAllKeyAssignmentReplacements
mpoke Dec 21, 2022
352a426
add expected order to TestGetAllConsumerAddrsToPrune
mpoke Dec 21, 2022
eca0844
Add test for GetSlashAndTrailingData (#623)
shaspitz Dec 21, 2022
78bcaae
use InitTimeoutTimestamp instead of two slices
mpoke Dec 21, 2022
253576a
Fix: Change keys for storing proposals (#620)
mpoke Dec 21, 2022
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
13 changes: 3 additions & 10 deletions proto/interchain_security/ccv/provider/v1/genesis.proto
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ message GenesisState {
(gogoproto.moretags) = "yaml:\"consumer_states\""
];
// empty for a new chain
repeated interchain_security.ccv.v1.UnbondingOp unbonding_ops = 3
repeated interchain_security.ccv.provider.v1.UnbondingOp unbonding_ops = 3
[ (gogoproto.nullable) = false ];
// empty for a new chain
interchain_security.ccv.v1.MaturedUnbondingOps mature_unbonding_ops = 4;
Expand Down Expand Up @@ -65,18 +65,11 @@ message ConsumerState {
repeated interchain_security.ccv.v1.ValidatorSetChangePacketData pending_valset_changes = 6
[ (gogoproto.nullable) = false ];
repeated string slash_downtime_ack = 7;
// UnbondingOpsIndex defines the unbonding operations on the consumer chain
repeated UnbondingOpIndex unbonding_ops_index = 8
// UnbondingOpsIndex defines the unbonding operations waiting on this consumer chain
repeated interchain_security.ccv.provider.v1.VscUnbondingOps unbonding_ops_index = 8
[ (gogoproto.nullable) = false ];
}

// UnbondingOpIndex defines the genesis information for each unbonding operations index
// referenced by chain id and valset udpate id
message UnbondingOpIndex {
uint64 valset_update_id = 1;
repeated uint64 unbonding_op_index = 2;
}

// ValsetUpdateIdToHeight defines the genesis information for the mapping
// of each valset udpate id to a block height
message ValsetUpdateIdToHeight {
Expand Down
40 changes: 39 additions & 1 deletion proto/interchain_security/ccv/provider/v1/provider.proto
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import "google/protobuf/timestamp.proto";
import "google/protobuf/duration.proto";
import "ibc/core/client/v1/client.proto";
import "ibc/lightclients/tendermint/v1/tendermint.proto";
import "tendermint/crypto/keys.proto";

// ConsumerAdditionProposal is a governance proposal on the provider chain to spawn a new consumer chain.
// If it passes, then all validators on the provider chain are expected to validate the consumer chain at spawn time
Expand Down Expand Up @@ -153,4 +154,41 @@ message ConsumerRemovalProposals {
// AddressList contains a list of consensus addresses
message AddressList {
repeated bytes addresses = 1;
}
}

message ChannelToChain {
string channel_id = 1;
string chain_id = 2;
}

// VscUnbondingOps contains the IDs of unbonding operations that are waiting for
// at least one VSCMaturedPacket with vscID from a consumer chain
message VscUnbondingOps {
uint64 vsc_id = 1;
repeated uint64 unbonding_op_ids = 2;
}

// UnbondingOp contains the ids of consumer chains that need to unbond before
// the unbonding operation with the given ID can unbond
message UnbondingOp {
uint64 id = 1;
// consumer chains that are still unbonding
repeated string unbonding_consumer_chains = 2;
}

message InitTimeoutTimestamp {
string chain_id = 1;
uint64 timestamp = 2;
}

message VscSendTimestamp {
uint64 vsc_id = 1;
google.protobuf.Timestamp timestamp = 2 [(gogoproto.stdtime) = true, (gogoproto.nullable) = false];
}

message KeyAssignmentReplacement {
bytes provider_addr = 1;
tendermint.crypto.PublicKey prev_c_key = 2;
int64 power = 3;
}

12 changes: 0 additions & 12 deletions proto/interchain_security/ccv/v1/ccv.proto
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,6 @@ message ValidatorSetChangePackets {
];
}


message UnbondingOp {
uint64 id = 1;
// consumer chains that are still unbonding
repeated string unbonding_consumer_chains = 2;
}

// This packet is sent from the consumer chain to the provider chain
// to notify that a VSC packet reached maturity on the consumer chain.
message VSCMaturedPacketData {
Expand All @@ -60,11 +53,6 @@ message SlashPacketData {
cosmos.staking.v1beta1.InfractionType infraction = 3;
}

// UnbondingOpsIndex defines a list of unbonding operation ids.
message UnbondingOpsIndex {
repeated uint64 ids = 1;
}

// MaturedUnbondingOps defines a list of ids corresponding to ids of matured unbonding operations.
message MaturedUnbondingOps {
repeated uint64 ids = 1;
Expand Down
8 changes: 3 additions & 5 deletions tests/difference/core/driver/core_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -390,11 +390,9 @@ func (s *CoreSuite) TestAssumptions() {
s.Require().Empty(s.consumerKeeper().GetPendingPackets(s.ctx(C)))

// Consumer has no maturities
s.consumerKeeper().IteratePacketMaturityTime(s.ctx(C),
func(vscId uint64, timeNs uint64) (stop bool) {
s.T().Fatal(FAIL_MSG)
return false // Don't stop
})
for range s.consumerKeeper().GetAllPacketMaturityTimes(s.ctx(C)) {
s.T().Fatal(FAIL_MSG)
}

// Consumer power
for i := 0; i < len(initState.ValStates.Status); i++ {
Expand Down
23 changes: 9 additions & 14 deletions tests/e2e/stop_consumer.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,21 +170,16 @@ func (s *CCVTestSuite) checkConsumerChainIsRemoved(chainID string, checkChannel
}

// check UnbondingOps were deleted and undelegation entries aren't onHold
providerKeeper.IterateOverUnbondingOpIndex(
s.providerCtx(),
chainID,
func(vscID uint64, ubdIndex []uint64) (stop bool) {
_, found := providerKeeper.GetUnbondingOpIndex(s.providerCtx(), chainID, uint64(vscID))
for _, unbondingOpsIndex := range providerKeeper.GetAllUnbondingOpIndexes(s.providerCtx(), chainID) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I can't really review this logic without more context or comments. It looks like nothing changed besides

IterateOverUnbondingOpIndex becoming GetAllUnbondingOpIndexes ?

_, found := providerKeeper.GetUnbondingOpIndex(s.providerCtx(), chainID, unbondingOpsIndex.VscId)
s.Require().False(found)
for _, ubdID := range unbondingOpsIndex.UnbondingOpIds {
_, found = providerKeeper.GetUnbondingOp(s.providerCtx(), unbondingOpsIndex.UnbondingOpIds[ubdID])
s.Require().False(found)
for _, ubdID := range ubdIndex {
_, found = providerKeeper.GetUnbondingOp(s.providerCtx(), ubdIndex[ubdID])
s.Require().False(found)
ubd, _ := providerStakingKeeper.GetUnbondingDelegationByUnbondingID(s.providerCtx(), ubdIndex[ubdID])
s.Require().Zero(ubd.Entries[ubdID].UnbondingOnHoldRefCount)
}
return false // do not stop the iteration
},
)
ubd, _ := providerStakingKeeper.GetUnbondingDelegationByUnbondingID(s.providerCtx(), unbondingOpsIndex.UnbondingOpIds[ubdID])
s.Require().Zero(ubd.Entries[ubdID].UnbondingOnHoldRefCount)
}
}

// verify consumer chain's states are removed
_, found := providerKeeper.GetConsumerGenesis(s.providerCtx(), chainID)
Expand Down
11 changes: 11 additions & 0 deletions testutil/keeper/unit_test_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -254,3 +254,14 @@ func GetTestConsumerAdditionProp() *providertypes.ConsumerAdditionProposal {

return prop
}

// Obtains a CrossChainValidator with a newly generated key, and randomized field values
func GetNewCrossChainValidator(t *testing.T) consumertypes.CrossChainValidator {
b1 := make([]byte, 8)
_, _ = rand.Read(b1)
power := int64(binary.BigEndian.Uint64(b1))
privKey := ed25519.GenPrivKey()
validator, err := consumertypes.NewCCValidator(privKey.PubKey().Address(), power, privKey.PubKey())
require.NoError(t, err)
return validator
}
37 changes: 4 additions & 33 deletions x/ccv/consumer/keeper/genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,35 +118,6 @@ func (k Keeper) ExportGenesis(ctx sdk.Context) (genesis *consumertypes.GenesisSt
panic("provider client does not exist")
}

maturingPackets := []consumertypes.MaturingVSCPacket{}
k.IteratePacketMaturityTime(ctx, func(vscId, timeNs uint64) (stop bool) {
mat := consumertypes.MaturingVSCPacket{
VscId: vscId,
MaturityTime: timeNs,
}
maturingPackets = append(maturingPackets, mat)
return false // do not stop the iteration
})

heightToVCIDs := []consumertypes.HeightToValsetUpdateID{}
k.IterateHeightToValsetUpdateID(ctx, func(height, vscID uint64) (stop bool) {
hv := consumertypes.HeightToValsetUpdateID{
Height: height,
ValsetUpdateId: vscID,
}
heightToVCIDs = append(heightToVCIDs, hv)
return false // do not stop the iteration
})

outstandingDowntimes := []consumertypes.OutstandingDowntime{}
k.IterateOutstandingDowntime(ctx, func(addr string) (stop bool) {
od := consumertypes.OutstandingDowntime{
ValidatorConsensusAddress: addr,
}
outstandingDowntimes = append(outstandingDowntimes, od)
return false // do not stop the iteration
})

// TODO: update GetLastTransmissionBlockHeight to not return an error
ltbh, err := k.GetLastTransmissionBlockHeight(ctx)
if err != nil {
Expand All @@ -156,11 +127,11 @@ func (k Keeper) ExportGenesis(ctx sdk.Context) (genesis *consumertypes.GenesisSt
genesis = consumertypes.NewRestartGenesisState(
clientID,
channelID,
maturingPackets,
k.GetAllPacketMaturityTimes(ctx),
valset,
k.GetHeightToValsetUpdateIDs(ctx),
k.GetAllHeightToValsetUpdateIDs(ctx),
k.GetPendingPackets(ctx),
outstandingDowntimes,
k.GetAllOutstandingDowntimes(ctx),
*ltbh,
params,
)
Expand All @@ -178,7 +149,7 @@ func (k Keeper) ExportGenesis(ctx sdk.Context) (genesis *consumertypes.GenesisSt
"",
nil,
valset,
k.GetHeightToValsetUpdateIDs(ctx),
k.GetAllHeightToValsetUpdateIDs(ctx),
k.GetPendingPackets(ctx),
nil,
consumertypes.LastTransmissionBlockHeight{},
Expand Down
12 changes: 6 additions & 6 deletions x/ccv/consumer/keeper/genesis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ func TestInitGenesis(t *testing.T) {
require.Equal(t, vscID, ck.GetPacketMaturityTime(ctx, vscID))
require.Equal(t, pendingDataPackets, ck.GetPendingPackets(ctx))

require.Equal(t, gs.OutstandingDowntimeSlashing, ck.GetOutstandingDowntimes(ctx))
require.Equal(t, gs.OutstandingDowntimeSlashing, ck.GetAllOutstandingDowntimes(ctx))

ltbh, err := ck.GetLastTransmissionBlockHeight(ctx)
require.NoError(t, err)
Expand Down Expand Up @@ -382,10 +382,10 @@ func assertProviderClientID(t *testing.T, ctx sdk.Context, ck *consumerkeeper.Ke
// assert that the given input match the height to valset update ID mappings in the store
func assertHeightValsetUpdateIDs(t *testing.T, ctx sdk.Context, ck *consumerkeeper.Keeper, heighValsetUpdateIDs []consumertypes.HeightToValsetUpdateID) {
ctr := 0
ck.IterateHeightToValsetUpdateID(ctx, func(height uint64, vscID uint64) (stop bool) {
require.Equal(t, heighValsetUpdateIDs[ctr].Height, height)
require.Equal(t, heighValsetUpdateIDs[ctr].ValsetUpdateId, vscID)

for _, heightToValsetUpdateID := range ck.GetAllHeightToValsetUpdateIDs(ctx) {
require.Equal(t, heighValsetUpdateIDs[ctr].Height, heightToValsetUpdateID.Height)
require.Equal(t, heighValsetUpdateIDs[ctr].ValsetUpdateId, heightToValsetUpdateID.ValsetUpdateId)
ctr++
return false
})
}
}
Loading