Skip to content

Commit 4de0b76

Browse files
committed
fix(evm): propagate a broken endorser registration instead of only logging it
registerEndorser swallowed every failure past the second-network and empty-allowlist checks (an unusable signing key, allowlist resolution, authorizer or responder construction, view-registry registration) as a log line. installEndorsement and Driver.New always returned success regardless, so a node explicitly configured with endorser.enabled came up looking healthy while never answering an endorsement request - discoverable only once a quorum it was needed for timed out, with nothing connecting the timeout back to the startup log. registerEndorser now returns an error that installEndorsement and New propagate, the same contract a broken submitter key already gets in newSubmitter. registeredFor is still only set once every step succeeds, so this does not change the existing retry-on-later-call or refuse-a-second-network behavior. Signed-off-by: atharrva01 <atharvaborade568@gmail.com>
1 parent 44a67a0 commit 4de0b76

2 files changed

Lines changed: 40 additions & 19 deletions

File tree

x/token/services/network/evm/driver.go

Lines changed: 21 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -312,7 +312,9 @@ func (d *Driver) installEndorsement(n *Network, config *Config, evmClient client
312312
// Registration happens now, not on the first approval. An endorser node answers requests without
313313
// ever making one, so registering lazily on the approval path would mean it never registers at
314314
// all and every request to it times out.
315-
d.registerEndorser(network+":"+channel, factory, config)
315+
if err := d.registerEndorser(network+":"+channel, factory, config); err != nil {
316+
return errors.Wrap(err, "evm: failed to register this node as an endorser")
317+
}
316318

317319
return nil
318320
}
@@ -350,11 +352,19 @@ func (d *Driver) newSubmitter(config *Config, evmClient client.EVMClient) (*Subm
350352
// against the wrong chain, so it is refused loudly here instead of silently discarded: an operator who
351353
// configures two endorsing networks on one node needs to see why the second one never answers.
352354
//
355+
// Because a broken endorser configuration answers no requests and looks identical to network trouble
356+
// from the outside, discoverable only once a quorum times out, every failure past that check is
357+
// returned to the caller instead of only logged: a node explicitly configured with endorser.enabled
358+
// must not start up looking healthy while unable to fulfil that role, the same contract newSubmitter
359+
// already holds for a configured-but-broken submitter key. registeredFor is set only once every step
360+
// succeeds, so a failed attempt does not permanently lock the node out of registering on a later,
361+
// successful call for the same network.
362+
//
353363
// The TMS is resolved when a request arrives rather than now: resolving one here would ask the token
354364
// layer for a service that is still being built through this very driver.
355-
func (d *Driver) registerEndorser(networkKey string, factory *endorsement.ServiceFactory, config *Config) {
365+
func (d *Driver) registerEndorser(networkKey string, factory *endorsement.ServiceFactory, config *Config) error {
356366
if d.viewRegistry == nil || !config.Endorser.Enabled {
357-
return
367+
return nil
358368
}
359369

360370
d.registerMu.Lock()
@@ -367,40 +377,32 @@ func (d *Driver) registerEndorser(networkKey string, factory *endorsement.Servic
367377
networkKey, d.registeredFor, networkKey)
368378
}
369379

370-
return
380+
return nil
371381
}
372382

373383
signer, err := config.EndorserSigner()
374384
if err != nil || signer == nil {
375-
logger.Errorf("this node is configured as an endorser but its key is unusable: %v", err)
376-
377-
return
385+
return errors.Wrap(err, "this node is configured as an endorser but its key is unusable")
378386
}
379387
allowed, err := config.AllowedRequesters(d.resolveIdentity)
380388
if err != nil {
381-
logger.Errorf("failed to resolve the endorsement allowlist: %v", err)
382-
383-
return
389+
return errors.Wrap(err, "failed to resolve the endorsement allowlist")
384390
}
385391
authorizer, err := endorsement.NewAuthorizer(allowed)
386392
if err != nil {
387-
logger.Errorf("failed to build the endorsement allowlist: %v", err)
388-
389-
return
393+
return errors.Wrap(err, "failed to build the endorsement allowlist")
390394
}
391395
responder, err := factory.NewResponder(authorizer, signer, d.resolveTMS)
392396
if err != nil {
393-
logger.Errorf("failed to build the endorsement responder: %v", err)
394-
395-
return
397+
return errors.Wrap(err, "failed to build the endorsement responder")
396398
}
397399
if err := endorsement.RegisterEndorser(d.viewRegistry, responder); err != nil {
398-
logger.Errorf("failed to register the endorsement responder: %v", err)
399-
400-
return
400+
return errors.Wrap(err, "failed to register the endorsement responder")
401401
}
402402
d.registeredFor = networkKey
403403
logger.Infof("registered as the endorser for [%s] with address %s", networkKey, signer.Address())
404+
405+
return nil
404406
}
405407

406408
// resolveIdentity turns a configured node name into the identity that node speaks with.

x/token/services/network/evm/driver_test.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -245,3 +245,22 @@ func TestRegisterEndorserSkipsANonEndorsingNetwork(t *testing.T) {
245245
assert.Equal(t, 1, registry.calls)
246246
assert.Equal(t, "network-a:", d.registeredFor)
247247
}
248+
249+
// TestRegisterEndorserReturnsAnErrorForABrokenKey is the regression test for the finding that a node
250+
// explicitly configured as an endorser, but whose signing key cannot be loaded, used to register
251+
// nothing and only log the failure: nothing told the caller registration never happened, so
252+
// installEndorsement and Driver.New both reported success regardless. A broken endorser answers no
253+
// requests, which looks identical to ordinary network trouble from the outside and was otherwise
254+
// discoverable only once a quorum it was needed for timed out.
255+
func TestRegisterEndorserReturnsAnErrorForABrokenKey(t *testing.T) {
256+
registry := &fakeViewRegistry{}
257+
d := &Driver{viewRegistry: registry, identities: fakeIdentityProvider{}}
258+
config := endorserConfig(t)
259+
config.Endorser.Keystore = "" // unusable: LoadKey rejects an empty path
260+
factory := testServiceFactory(t, config)
261+
262+
err := d.registerEndorser("network-a:", factory, config)
263+
require.Error(t, err)
264+
assert.Zero(t, registry.calls, "a broken key must not reach the view registry")
265+
assert.Empty(t, d.registeredFor, "a failed attempt must not mark the network as registered")
266+
}

0 commit comments

Comments
 (0)