-
Notifications
You must be signed in to change notification settings - Fork 62
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
check potential error before Commit #484
Conversation
@@ -649,6 +649,10 @@ func (s *StateDB) clearJournalAndRefund() { | |||
|
|||
// Commit writes the state to the underlying in-memory trie database. | |||
func (s *StateDB) Commit(deleteEmptyObjects bool) (root common.Hash, err error) { | |||
if s.dbErr != nil { | |||
return common.Hash{}, fmt.Errorf("commit aborted due to earlier error: %v", s.dbErr) | |||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dbErr does not mean a real error, eg:
func (self *StateDB) GetCodeSize(addr common.Address) int {
stateObject := self.getStateObject(addr)
if stateObject == nil {
return 0
}
if stateObject.code != nil {
return len(stateObject.code)
}
size, err := self.db.ContractCodeSize(stateObject.addrHash, common.BytesToHash(stateObject.CodeHash()))
if err != nil {
self.setError(err)
}
return size
}
The function GetCodeSize
set dbErr when the account does not exist, but the return value size 0 is right.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would account not existing not be an error? I don't know the repo deep enough about this.
In go-ethereum GetCodeSize is handled the same way. Using setError
instead of the usual return nil, err
pattern.
func (s *StateDB) GetCodeSize(addr common.Address) int {
stateObject := s.getStateObject(addr)
if stateObject != nil {
return stateObject.CodeSize()
}
return 0
}
func (s *stateObject) CodeSize() int {
if s.code != nil {
return len(s.code)
}
if bytes.Equal(s.CodeHash(), types.EmptyCodeHash.Bytes()) {
return 0
}
size, err := s.db.db.ContractCodeSize(s.address, common.BytesToHash(s.CodeHash()))
if err != nil {
s.db.setError(fmt.Errorf("can't load code size %x: %v", s.CodeHash(), err))
}
return size
}
This reverts commit 0114eca.
Proposed changes
The StateDB type utilizes a dbErr to track database errors. It is employed by both the consensus core and the VM which are not designed to manage such errors directly.
Errors encountered during database operations are recorded in dbErr using the SetError function. During a Commit, functions like deleteStateObject, updateStateObject, and getStateObject are called based on the current state of stateObject to manage trie node data for deletion, update, or retrieval. If a data node is missing, dbErr is set accordingly.
To avoid committing erroneous state, check the dbErr before performing Commit action.
This issue is raised by the auditing team
ref: https://github.com/ethereum/go-ethereum/pull/21039/files
Types of changes
What types of changes does your code introduce to XDC network?
Put an
✅
in the boxes that applyImpacted Components
Which part of the codebase this PR will touch base on,
Put an
✅
in the boxes that applyChecklist
Put an
✅
in the boxes once you have confirmed below actions (or provide reasons on not doing so) that