From ac38c9776c6bf39c81c2e4008a05b71c611deb2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Kripalani?= Date: Tue, 10 Jan 2023 18:24:52 +0000 Subject: [PATCH] add a network version gate to IsValidForSending. --- chain/consensus/filcns/filecoin.go | 24 ++++++++++++++++++++++-- chain/messagepool/messagepool.go | 6 +++++- 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/chain/consensus/filcns/filecoin.go b/chain/consensus/filcns/filecoin.go index ec801ea85..3601fd1bc 100644 --- a/chain/consensus/filcns/filecoin.go +++ b/chain/consensus/filcns/filecoin.go @@ -436,15 +436,35 @@ func (filec *FilecoinEC) VerifyWinningPoStProof(ctx context.Context, nv network. return nil } -func IsValidForSending(act *types.Actor) bool { +func IsValidForSending(nv network.Version, act *types.Actor) bool { + // Before nv18 (Hygge), we only supported built-in account actors as senders. + // + // Note: this gate is probably superfluous, since: + // 1. Placeholder actors cannot be created before nv18. + // 2. EthAccount actors cannot be created before nv18. + // 3. Delegated addresses cannot be created before nv18. + // + // But it's a safeguard. + // + // Note 2: ad-hoc checks for network versions like this across the codebase + // will be problematic with networks with diverging version lineages + // (e.g. Hyperspace). We need to revisit this strategy entirely. + if nv < network.Version18 { + return builtin.IsAccountActor(act.Code) + } + + // After nv18, we also support other kinds of senders. if builtin.IsAccountActor(act.Code) || builtin.IsEthAccountActor(act.Code) { return true } + // Allow placeholder actors with a delegated address and nonce 0 to send a message. + // These will be converted to an EthAccount actor on first send. if !builtin.IsPlaceholderActor(act.Code) || act.Nonce != 0 || act.Address == nil || act.Address.Protocol() != address.Delegated { return false } + // Only allow such actors to send if their delegated address is in the EAM's namespace. id, _, err := varint.FromUvarint(act.Address.Payload()) return err == nil && id == builtintypes.EthereumAddressManagerActorID } @@ -521,7 +541,7 @@ func (filec *FilecoinEC) checkBlockMessages(ctx context.Context, b *types.FullBl return xerrors.Errorf("failed to get actor: %w", err) } - if !IsValidForSending(act) { + if !IsValidForSending(nv, act) { return xerrors.New("Sender must be an account actor") } nonces[sender] = act.Nonce diff --git a/chain/messagepool/messagepool.go b/chain/messagepool/messagepool.go index bff99b255..98d0e29ab 100644 --- a/chain/messagepool/messagepool.go +++ b/chain/messagepool/messagepool.go @@ -870,8 +870,12 @@ func (mp *MessagePool) addTs(ctx context.Context, m *types.SignedMessage, curTs return false, xerrors.Errorf("failed to get sender actor: %w", err) } + // This message can only be included in the _next_ epoch and beyond, hence the +1. + epoch := curTs.Height() + 1 + nv := mp.api.StateNetworkVersion(ctx, epoch) + // TODO: I'm not thrilled about depending on filcns here, but I prefer this to duplicating logic - if !filcns.IsValidForSending(senderAct) { + if !filcns.IsValidForSending(nv, senderAct) { return false, xerrors.Errorf("sender actor %s is not a valid top-level sender", m.Message.From) }