nspcc-dev/neo-go

Fork 0

mirror of https://github.com/nspcc-dev/neo-go.git synced 2026-03-01 04:28:51 +00:00

Optmize GetLatestStateHeight calls #1486

New issue

Open

opened 2025-12-28 17:16:36 +00:00 by sami · 5 comments

sami commented

2025-12-28 17:16:36 +00:00

Owner

Originally created by @lock9 on GitHub (Feb 18, 2025).

We are deploying our infrastructure using Neo Go. We noticed that the historic endpoints consume a lot of CPU resources. A few calls to historic endpoints are enough to 'break' the server. Maybe it's a configuration issue, but we have requests that stay over five minutes waiting for Neo Go.

The culprit seems to be the GetLatestStateHeight function:

nspcc-dev/neo-go@0d8c751e50/pkg/core/stateroot/module.go (L134)

// GetLatestStateHeight returns the latest blockchain height by the given stateroot.
func (s *Module) GetLatestStateHeight(root util.Uint256) (uint32, error) {
	rootBytes := root.BytesBE()
	rootStartOffset := 1 + 4 // stateroot version (1 byte) + stateroot index (4 bytes)
	rootEndOffset := rootStartOffset + util.Uint256Size
	var (
		h       uint32
		found   bool
		rootKey = makeStateRootKey(s.localHeight.Load())
	)
	s.Store.Seek(storage.SeekRange{
		Prefix:    []byte{rootKey[0]}, // DataMPTAux
		Start:     rootKey[1:],        // Start is a value that should be appended to the Prefix
		Backwards: true,
	}, func(k, v []byte) bool {
		if len(k) == 5 && bytes.Equal(v[rootStartOffset:rootEndOffset], rootBytes) {
			h = binary.BigEndian.Uint32(k[1:]) // cut prefix DataMPTAux
			found = true
			return false
		}
		return true
	})
	if found {
		return h, nil
	}
	return h, storage.ErrKeyNotFound
}

Describe the solution you'd like

Create a prefix to map roots to heights, getting the roots directly, without using 'seek'. Adding this new information has a few downsides:

One extra write
Around ~41 extra bytes per block (prefix + root + height) = 1 + 32 + 8 (or 4?)

However, this would significantly speed up historical requests and prevent excessive CPU consumption. In practice, CPU usage became so intense that we struggled to connect to our VMs. IMHO, the performance improvement outweighs the minor increase in storage usage.

EDIT: Sorry for the confusion, I'm not sure if it's either height -> root or root -> height

Describe alternatives you've considered

I've tried a 'read cache', but that doesn't solve the issue. We also have an indexer being built.

Additional context

I implemented this feature in a local branch. It does improve the performance, but it's not fully tested. Maybe it doesn't work.

Don't forget to add labels!

engine
enhancement

Originally created by @lock9 on GitHub (Feb 18, 2025). ## Is your feature request related to a problem? Please describe. We are deploying our infrastructure using Neo Go. We noticed that the historic endpoints consume a lot of CPU resources. A few calls to historic endpoints are enough to 'break' the server. Maybe it's a configuration issue, but we have requests that stay over five minutes waiting for Neo Go. The culprit seems to be the `GetLatestStateHeight` function: https://github.com/nspcc-dev/neo-go/blob/0d8c751e50951ad84cec8b89cf6ea1a9e0f4f878/pkg/core/stateroot/module.go#L134 ```go // GetLatestStateHeight returns the latest blockchain height by the given stateroot. func (s *Module) GetLatestStateHeight(root util.Uint256) (uint32, error) { rootBytes := root.BytesBE() rootStartOffset := 1 + 4 // stateroot version (1 byte) + stateroot index (4 bytes) rootEndOffset := rootStartOffset + util.Uint256Size var ( h uint32 found bool rootKey = makeStateRootKey(s.localHeight.Load()) ) s.Store.Seek(storage.SeekRange{ Prefix: []byte{rootKey[0]}, // DataMPTAux Start: rootKey[1:], // Start is a value that should be appended to the Prefix Backwards: true, }, func(k, v []byte) bool { if len(k) == 5 && bytes.Equal(v[rootStartOffset:rootEndOffset], rootBytes) { h = binary.BigEndian.Uint32(k[1:]) // cut prefix DataMPTAux found = true return false } return true }) if found { return h, nil } return h, storage.ErrKeyNotFound } ``` ## Describe the solution you'd like Create a prefix to map roots to heights, getting the roots directly, without using 'seek'. Adding this new information has a few downsides: - One extra write - Around ~41 extra bytes per block (prefix + root + height) = 1 + 32 + 8 (or 4?) However, this would significantly speed up historical requests and prevent excessive CPU consumption. In practice, CPU usage became so intense that we struggled to connect to our VMs. IMHO, the performance improvement outweighs the minor increase in storage usage. EDIT: Sorry for the confusion, I'm not sure if it's either height -> root or root -> height ## Describe alternatives you've considered I've tried a 'read cache', but that doesn't solve the issue. We also have an indexer being built. ## Additional context I implemented this feature in a local branch. It does improve the performance, but it's not fully tested. Maybe it doesn't work. ## Don't forget to add labels! - engine - enhancement

sami added the

enhancement

performance

labels

2025-12-28 17:16:36 +00:00

sami commented

2025-12-28 17:16:37 +00:00

Author

Owner

@roman-khimov commented on GitHub (Feb 19, 2025):

How exactly are you doing these calls? The only case where GetLatestStateHeight is invoked at all is when you're using state hash as parameter. But in fact you can use block index or block hash and avoid this function being called completely. In the vast majority of cases block indexes or hashes are more available to users than state root hashes.

@roman-khimov commented on GitHub (Feb 19, 2025): How exactly are you doing these calls? The only case where `GetLatestStateHeight` is invoked at all is when you're using state hash as parameter. But in fact you can use block index or block hash and avoid this function being called completely. In the vast majority of cases block indexes or hashes are more available to users than state root hashes.

sami commented

2025-12-28 17:16:37 +00:00

Author

Owner

@lock9 commented on GitHub (Feb 19, 2025):

We use it to get historical balances (most times). In fact, what you said is very relevant, becaus it was a bug in the frontend that was sending incorrect parameters (root hash). This was making code fall into the GetLatestStateHeight branch. This may explain why caching wasn't working as expected.

The 'problem' with this code, is that a few requests is enough to consume all the CPU available. Are these extra branches necessary? (accept the height only instead of the root)

I know that the current code was querying for the state root, and then making the historic call. If I understand correctly, the first step is skippable.

func (s *Server) getHistoricParams(reqParams params.Params) (uint32, *neorpc.Error) {
	if s.chain.GetConfig().Ledger.KeepOnlyLatestState {
		return 0, neorpc.WrapErrorWithData(neorpc.ErrUnsupportedState, fmt.Sprintf("only latest state is supported: %s", errKeepOnlyLatestState))
	}
	if len(reqParams) < 1 {
		return 0, neorpc.ErrInvalidParams
	}
	height, respErr := s.blockHeightFromParam(reqParams.Value(0))
	if respErr != nil {
		hash, err := reqParams.Value(0).GetUint256()
		if err != nil {
			return 0, neorpc.NewInvalidParamsError(fmt.Sprintf("invalid block hash or index or stateroot hash: %s", err))
		}
		b, err := s.chain.GetBlock(hash)
		if err != nil {
			stateH, err := s.chain.GetStateModule().GetLatestStateHeight(hash) <----- HERE
			if err != nil {
				return 0, neorpc.NewInvalidParamsError(fmt.Sprintf("unknown block or stateroot: %s", err))
			}
			height = stateH
		} else {
			height = b.Index
		}
	}
	return height + 1, nil
}

@lock9 commented on GitHub (Feb 19, 2025): We use it to get historical balances (most times). In fact, what you said is very relevant, becaus it was a bug in the frontend that was sending incorrect parameters (root hash). This was making code fall into the `GetLatestStateHeight` branch. This may explain why caching wasn't working as expected. The 'problem' with this code, is that a few requests is enough to consume all the CPU available. Are these extra branches necessary? (accept the height only instead of the root) I know that the current code *was* querying for the state root, and then making the historic call. If I understand correctly, the first step is skippable. ```go func (s *Server) getHistoricParams(reqParams params.Params) (uint32, *neorpc.Error) { if s.chain.GetConfig().Ledger.KeepOnlyLatestState { return 0, neorpc.WrapErrorWithData(neorpc.ErrUnsupportedState, fmt.Sprintf("only latest state is supported: %s", errKeepOnlyLatestState)) } if len(reqParams) < 1 { return 0, neorpc.ErrInvalidParams } height, respErr := s.blockHeightFromParam(reqParams.Value(0)) if respErr != nil { hash, err := reqParams.Value(0).GetUint256() if err != nil { return 0, neorpc.NewInvalidParamsError(fmt.Sprintf("invalid block hash or index or stateroot hash: %s", err)) } b, err := s.chain.GetBlock(hash) if err != nil { stateH, err := s.chain.GetStateModule().GetLatestStateHeight(hash) <----- HERE if err != nil { return 0, neorpc.NewInvalidParamsError(fmt.Sprintf("unknown block or stateroot: %s", err)) } height = stateH } else { height = b.Index } } return height + 1, nil } ```

sami commented

2025-12-28 17:16:37 +00:00

Author

Owner

@roman-khimov commented on GitHub (Feb 19, 2025):

Are these extra branches necessary?

Yes because we always wanted to give more options to users. Therefore we support block numbers, block hashes and state root hashes as parameters for historic calls.

Now if that's really problematic wrt resource use we may think of omitting state root hash support, although it's somewhat natural for state-related queries.

I know that the current code was querying for the state root, and then making the historic call. If I understand correctly, the first step is skippable.

That's exactly what I'm trying to convey, in many cases you don't need state root hash itself.

@roman-khimov commented on GitHub (Feb 19, 2025): > Are these extra branches necessary? Yes because we always wanted to give more options to users. Therefore we support block numbers, block hashes and state root hashes as parameters for historic calls. Now if that's really problematic wrt resource use we may think of omitting state root hash support, although it's somewhat natural for state-related queries. > I know that the current code was querying for the state root, and then making the historic call. If I understand correctly, the first step is skippable. That's exactly what I'm trying to convey, in many cases you don't need state root hash itself.

sami commented

2025-12-28 17:16:37 +00:00

Author

Owner

@lock9 commented on GitHub (Feb 20, 2025):

If sending an inexistent hash causes the node to read 'all the storage', it must either be optimized or disabled. The first option seems more reasonable since removing it may cause applications to break.

Do you think it's possible to optimize it without consuming more storage or using an extra write? Maybe using an in-memory bloom filter? It won't solve the issue 'permanently', but it could prevent the node from being stuck on all requests

@lock9 commented on GitHub (Feb 20, 2025): If sending an inexistent hash causes the node to read 'all the storage', it must either be optimized or disabled. The first option seems more reasonable since removing it may cause applications to break. Do you think it's possible to optimize it without consuming more storage or using an extra write? Maybe using an in-memory bloom filter? It won't solve the issue 'permanently', but it could prevent the node from being stuck on all requests

sami commented

2025-12-28 17:16:37 +00:00

Author

Owner

@roman-khimov commented on GitHub (Feb 20, 2025):

I doubt it can be solved without DB scheme change.

@roman-khimov commented on GitHub (Feb 20, 2025): I doubt it can be solved without DB scheme change.