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

add slash throttling related queries #563

Closed
wants to merge 7 commits into from

Conversation

MSalopek
Copy link
Contributor

@MSalopek MSalopek commented Dec 6, 2022

Description

This PR adds 2 new query methods on the provider

interchain-security-pd q provider pending-consumer-packets <chain_id>

  • lists all pending data packets (both slash and VSC matured) for chain_id
  • Eg.
{
    "chain_id": "consu",
    "packets": [],
    "size": "0"
}

interchain-security-pd q provider pending-slash-packets

  • lists all pending slash packets with some data about the slash meter
  • Eg.
{
    "slash_meter": "50",
    "slash_meter_allowance": "75",
    "last_replenish": "2022-12-06T14:47:14.249756250Z",
    "packets": []
}

Linked issues

Closes: #562

Type of change

Please delete options that are not relevant.

  • Non-breaking changes
  • New feature (adding functionality)
  • Proto files changes

Checklist:

Please delete options that are not relevant.

  • Relevant issus are linked
  • Tests are passing (make test)
  • make proto-gen was run (for changes in .proto files)

@MSalopek MSalopek force-pushed the masa/562-throttle-queries branch from bb4ec50 to 3635e2a Compare December 6, 2022 17:46
@MSalopek MSalopek changed the base branch from circuit-breaker to goc-december December 6, 2022 17:50
@jtremback
Copy link
Contributor

Looks OK, will have to review it further- Have you ever been able to test this out? What does the serialization look like with fuller queues? Can you add an integration test?

[]byte(packetEntry.ConsumerChainID),
)
}

// ParsePendingSlashPacketEntryKey returns the received time and chainID for a pending slash packet entry key
func ParsePendingSlashPacketEntryKey(bz []byte) (time.Time, string) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you explain more about why this needed to be changed?

Copy link
Contributor Author

@MSalopek MSalopek Dec 7, 2022

Choose a reason for hiding this comment

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

There's 2 queues:

  1. global queue holding SlashPacketEntry
  2. per-chain queue holding both VSCMaturedPacketData and SlashPacketData

When querying the global queue, the data in it has no value to validators. SlashPacketEntry is just this:

// A persisted queue entry indicating that a slash packet data instance needs to be handled
type SlashPacketEntry struct {
	RecvTime time.Time
	ConsumerChainID string
	// IbcSeqNum uint64 -> not available in previous goc-december
	ValAddr []byte
}

It was brought to my attention that this data structure is not informative enough for the normal user since it is used for internal processing. Instead, it is better to provide the actual slash packet data (as transferred over IBC) since it contains useful information relevant to the user.

type SlashPacketData struct {
	Validator types.Validator
	ValsetUpdateId uint64
	Infraction types1.InfractionType
}

SlashPacketData is not available in the global queue -> we need to get it from the per-chain queue.

It was not possible to get it from the per-chain queue without adding the newer changes from circuit-breaker branch. The changes in question expand the per-chain queue key and add IbcSeq to SlashPacketEntry. With all data readily available we can fetch the exact slash packet that's waiting to be processed (validator, infraction, valsetUpdateId) and enrich the query output

TL;DR:
In the goc-december version of throttling we did not save the IBC sequence number inside the SlashPacketEntry in the global queue, so we could not connect the SlashPacketEntry to the per-chain SlashPacketData. By including the newer key version in this release we get more informative output from the query (the validator object, infraction and valsetupdateId)

{
	"chain_id": "foochain",
	"received_at": "07-12-2022 00:00:00Z",
	"data": {
		"validator": { ... },
		"valset_update_id": 12,
		"infraction": 2, // double sign
	}
}

vs.

{
	"chain_id": "foochain",
	"received_at": "07-12-2022 00:00:00Z",
	"validator": []bytes,
}

@MSalopek
Copy link
Contributor Author

MSalopek commented Dec 7, 2022

Looks OK, will have to review it further- Have you ever been able to test this out? What does the serialization look like with fuller queues? Can you add an integration test?

Tested on chains running in integration tests.
Adding integration tests is a bit tricky. I'm trying to add them

@mpoke mpoke marked this pull request as draft December 7, 2022 16:22
@MSalopek
Copy link
Contributor Author

Closing in favour of: #600.

This branch is too far out of date to be worth the effort.

@MSalopek MSalopek closed this Dec 14, 2022
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.

3 participants