refactor(api): remove dead WalletExtension gRPC service and config - #6975
0xbigapple wants to merge 1 commit into
Conversation
The four WalletExtension RPCs have returned UNIMPLEMENTED since 2019; the service was only registered on solidity nodes behind node.walletExtensionApi, which config.conf enabled but reference.conf disabled. Remove the service, its now-unreferenced messages (including the TimeMessage/TimePaginatedMessage orphans left by the 2018 RPC removal), the config key, and the dead client/test helpers. Log a removal warning when the old key is still present in operator configs.
| } | ||
|
|
||
| // node.walletExtensionApi (removed): the WalletExtension gRPC service no longer exists | ||
| if (section.hasPath("walletExtensionApi")) { |
There was a problem hiding this comment.
[NIT] First node.* key removed without a deprecation period — please record the exception in release notes
node.walletExtensionApi was a published config key (present in both shipped samples: config.conf defaulted it to true, reference.conf to false). The project config convention calls for a full deprecation cycle before removing a published key; this PR removes it in one step, with only this warn-and-ignore fallback. The practical impact is nil — old configs still boot (unknown keys are ignored) and operators get a clear warning, and ArgsTest.testRemovedWalletExtensionApiKeyIsIgnored locks that behavior in — so this reads as a documentable exception rather than a contract violation.
Suggestion: list the removal explicitly in the release notes ("Removed: WalletExtension gRPC service (4 RPCs) + node.walletExtensionApi config key") so the skipped deprecation cycle is visible to operators and future reviewers.
| nc.maxConnectionsWithSameIp = section.getInt("maxActiveNodesWithSameIp"); | ||
| } | ||
|
|
||
| // node.walletExtensionApi (removed): the WalletExtension gRPC service no longer exists |
There was a problem hiding this comment.
[NIT] PR description reflection claim doesn't match the code
(Cross-cutting note anchored at the removal warning as the most representative added line.)
The PR description says the only observable effect of enabling node.walletExtensionApi was that "gRPC reflection advertises a service that always fails", but this repo does not register ProtoReflectionService (no grpc-reflection dependency, no registration in RpcApiService). The actual observable effect is narrower: the solidity node registers a service whose every method returns UNIMPLEMENTED.
Suggestion: correct that sentence in the PR description, since it may be quoted in future decisions.
3 participants
What does this PR do?
close #6931.
Removes the dead
WalletExtensiongRPC service and everything reachable only from it:api.proto:service WalletExtension(GetTransactionsFromThis/2,GetTransactionsToThis/2), plus messagesAccountPaginated,TransactionList,TransactionListExtention(referenced only by these four RPCs) andTimeMessage/TimePaginatedMessage(request types of the WalletExtension*ByTimestampRPCs deleted in 2018, orphaned ever since)RpcApiService: the registration branch and the emptyWalletExtensionApiinner classnode.walletExtensionApiconfig item:CommonParameter/NodeConfigfields, theArgsbinding, and the key inreference.conf/config.conf/config-shield.conf. Following the retirement convention fornode.*keys,NodeConfig.fromConfignow logs a removal warning when the old key is still present in an operator configUtil.printTransactionList(sole caller was its own mock test), the WalletExtension stub and wrappers in test utilitiesGrpcClient/WalletClient,HttpMethed.getTransactions{From,To}ThisFromSolidity(targets/walletextension/*HTTP paths that have no servlet), and commented-outgetTransactionsByTimestamp/getAssetIssueListByTimestampblocksWhy are these changes required?
WalletExtensionhas had no implementation in any release since v3.7 (2020-03):RpcApiService$WalletExtensionApioverrides none of the four RPCs, so every call falls through to the generatedImplBasedefault handlers and returnsUNIMPLEMENTED. This makesnode.walletExtensionApibehavior-irrelevant — enabled, it registers a service with zero implemented methods; disabled, callers get the sameUNIMPLEMENTED. The only observable effect of enabling it is that gRPC reflection advertises a service that always fails. Removing it also resolves the default-value inconsistency betweenconfig.conf(true) andreference.conf(false).Six years of unconditional
UNIMPLEMENTEDrules out any functional dependency, so the service is removed directly without a deprecation period, following existing practice for dead interfaces.This PR has been tested by:
Follow up
Ecosystem code that still compiles against the removed stubs/messages needs a sync: the
tronprotocol/protocolmirror, the documentation site, and older wallet-cli/trident versions. Compile-time impact only — runtime behavior is unchanged (UNIMPLEMENTEDbefore and after).Extra details
None.