Repository navigation
feat(action): add IIP-59 voter reward actions and BLS candidate fields - #38
Merged
Merged
Conversation
The vendored proto stopped at ActionCore field 53, so this SDK could not build any Zanzibar action and silently dropped the BLS fields on candidate registration. Added to proto/types/action.proto, matched field-by-field against iotex-proto v0.6.13: candidateDeactivate (54), scheduleCandidateDeactivation (55), setVoterRewardOptIn (57), setVoterRewardDestination (58), and blsPubKey/blsPop on CandidateBasicInfo. Wrappers are provided for the two IIP-59 actions; the other two are proto-only for now. setCommissionRate (56) is deliberately absent. It exists in iotex-proto v0.6.13 but no core release implements it -- iotex-proto#168 is still open, and a testnet node answers "no applicable action to handle proto type" for it. Envelop needed a different presence check for the opt-in. SetVoterRewardOptIn carries no fields -- the sender is the delegate -- so it serializes to zero bytes, and the toByteArray().length > 0 test every other branch uses is false for it. Deserializing an opt-in through that path drops it, turning a delegate's one-way action into a silent no-op. The new branches read the oneof presence bit instead. The destination is decoded from io1... to its 20 raw bytes before it goes on the wire, because the protocol reads it with address.FromBytes; sending the bech32 text produces an action that signs and is then rejected on decode. Generated code was regenerated with protoc 34.1 to match the "Protobuf Java Version: 4.34.1" already in the tree and the protobuf-java 4.34.1 the pom pins. The local default is 35.1, whose gencode fails the runtime's version check. Four tests cover the two failure modes that are invisible until a node sees them: which oneof field each action occupies, and that the empty opt-in survives an Envelop round trip. Full suite has the same 9 pre-existing errors as master, in the same four network- and key-dependent classes.
The wire encoding was previously argued from antenna-go's TestNet run rather than observed from Java. This closes that, and exercises the TX_CONTAINER signing path PR #34 introduced. Both actions were submitted from a funded non-delegate account. Receipts at height 47,511,663: SetVoterRewardDestination status 1 (Success) SetVoterRewardOptIn status 205 (ErrCandidateNotExist) 205 is the outcome to want here, not a problem. The action reached the staking handler and was rejected on a protocol rule -- the sender is not a candidate -- which proves it was decoded and dispatched correctly. A wrapper that filed the action under the wrong oneof field would have failed earlier and differently, with "no applicable action to handle proto type", the way SetCommissionRate does. The opt-in was deliberately not sent from a delegate. It is one-way: the protocol has no counterpart action to opt back out, so running it against a real delegate would have permanently changed it to prove a point about encoding. TestNet's endorsetest was verified unchanged afterwards. The test skips unless IOTEX_TEST_KEY is set, so it is inert in CI and in any checkout without a funded key. No key is committed.
The endpoint and chain id were compiled in, so the only chain this test could reach was TestNet -- where SetVoterRewardOptIn is one-way and a run therefore has to be aimed at a non-delegate to stay safe, which means the opt-in path is only ever exercised as a rejection. IOTEX_TEST_ENDPOINT / IOTEX_TEST_CHAINID / IOTEX_TEST_SECURE point it at a local Zanzibar chain instead, where the fork activates at a two-digit height, accounts are disposable and a run costs nothing. IOTEX_TEST_SECURE=false is needed because a local node speaks plaintext gRPC while TestNet's :443 does not. Defaults are unchanged, so an existing invocation still goes to TestNet. Verified against a local four-delegate chain on iotex-core v2.5.0 with Zanzibar, Beta and Gamma all active: both actions were accepted.
The previous commit added blsPubKey and blsPop to the generated
CandidateBasicInfo, but nothing above it could set them: neither
CandidateRegisterRequest nor CandidateUpdateRequest had the fields, and neither
method ever wrote them. A Java caller could not attach a BLS key at all. It is
optional after Zanzibar, so registering and updating kept working -- which is
why the gap was not visible -- but key rotation was impossible, and IIP-52's
signature aggregation makes that a hard requirement rather than a nicety.
Both requests now carry the pair as hex, and both methods build the message
through one shared AbstractMethod.candidateBasicInfo. That consolidation is
part of the fix, not tidying: each method built CandidateBasicInfo twice, once
in execute() and once in signedAction(), from two copies of the same code. A
field added to one copy and not the other yields an action that is right when
sent and wrong when signed offline, with nothing on either path to notice.
Three behaviours worth stating:
- A 0x prefix is stripped. Numeric.hexStringToByteArray does not do it --
it reads '0' and 'x' as a byte, and Character.digit('x', 16) is -1 -- so
a prefixed key decodes to something corrupt of a plausible length, signs
cleanly, and is rejected on chain with no indication why. Every tool that
emits a BLS key quotes it with the prefix.
- Absent stays absent. A null or empty field is left unset rather than set
to empty bytes, so an update that does not mention a key leaves an already
registered one alone.
- A proof with no key throws. That pairing is invalid in every fork era and
the chain rejects it at validation, before a receipt exists, so the caller
would get a bare RPC error with no action hash to look up.
CandidateBLSLiveTest sends candidateUpdate twice against a running chain: with
a proof bound to the candidate's own owner address, and with one bound to a
different candidate. The negative case is the point -- a wiring that swapped
the two fields, truncated one, or lost the prefix would satisfy the unit tests
and be rejected by the chain identically. Only the pair separates "accepted"
from "verified".
It also reads the transaction logs of a settlement block. IIP-59 adds no new
TransactionLogType -- voter payouts reuse CLAIM_FROM_REWARDING_FUND -- so what
that checks is that the sender survives: it is the protocol pool
pseudo-address io0000000000000000000000rewardingprotocol, which has no 20-byte
hash behind it and comes back empty from anything that decodes an address the
ordinary way.
Verified against a local four-delegate chain on iotex-core v2.5.0 with
Zanzibar, Beta and Gamma active: status 1 for the bound proof, 203
(ErrUnauthorizedOperator) for the foreign one, and three payout logs decoded.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The vendored proto stopped at
ActionCorefield 53, so this SDK could not build any Zanzibar action and silently dropped the BLS fields on candidate registration.What's added
Matched field-by-field against
iotex-proto v0.6.13:candidateDeactivatescheduleCandidateDeactivationsetVoterRewardOptInsetVoterRewardDestinationCandidateBasicInfo.blsPubKey/blsPopTwo things worth reviewing
SetVoterRewardOptInneeded a different presence check. It carries no fields — the sender is the delegate — so it serializes to zero bytes, and thetoByteArray().length > 0test every otherEnvelopbranch uses is false for it. Deserializing an opt-in through that path drops it, turning a delegate's one-way, irreversible action into a silent no-op. The new branches read the oneof presence bit instead. There's a test for exactly this.The destination goes on the wire as 20 raw bytes, decoded from
io1...via Bech32, because the protocol reads it withaddress.FromBytes. Sending the bech32 string produces an action that signs cleanly and is rejected on decode.setCommissionRate(56) is deliberately absentIt exists in iotex-proto v0.6.13, but no core release implements it — iotex-proto#168 is still open, and a testnet node answers
no applicable action to handle proto typefor it. I found this while doing the same work in antenna-go (#115), where I had already written the wrapper before a testnet round-trip caught it.Codegen
Regenerated with protoc 34.1, matching the
Protobuf Java Version: 4.34.1already in the tree and theprotobuf-java 4.34.1the pom pins. The local default here is 35.1, whose gencode fails protobuf-java's runtime version check — worth knowing before anyone regenerates.Testing
4 new tests, covering the two failure modes that are invisible until a node sees them: which oneof field each action occupies, and that the empty opt-in survives an
Envelopround trip.Full suite: same 9 pre-existing errors as master, in the same four classes (
ContractTest,KeystoreTest,IOTXTest,RawBlockTest— all network- or key-dependent). Verified by stashing this change and re-running.Not verified here
No testnet round trip from Java. antenna-go's equivalent actions were submitted to testnet and accepted (decoded and validated, rejected only for an empty balance), and the wire encoding is the same — but that is an inference, not a Java-side observation. If you want it closed properly, a signed submission from Java with a funded key is the missing step, and it would also exercise the
TX_CONTAINERsigning path that PR #34 introduced.