Fix for node removal from trie.

This commit is contained in:
Gav Wood 2015-12-03 00:32:58 +01:00
parent 3143e614a8
commit b99cefb9d6

View File

@ -589,29 +589,38 @@ impl TrieDB {
/// ///
/// **This operation will not insert the new node nor destroy the original.** /// **This operation will not insert the new node nor destroy the original.**
fn fixed<'a, 'b>(&'a self, n: Node<'b>, diff: &mut Diff) -> MaybeChanged<'b> where 'a: 'b { fn fixed<'a, 'b>(&'a self, n: Node<'b>, diff: &mut Diff) -> MaybeChanged<'b> where 'a: 'b {
trace!("fixed node={:?}", n);
match n { match n {
Node::Branch(nodes, node_value) => { Node::Branch(nodes, node_value) => {
// if only a single value, transmute to leaf/extension and feed through fixed. // if only a single value, transmute to leaf/extension and feed through fixed.
let mut index: [u8; 1] = [16; 1]; #[derive(Debug)]
enum UsedIndex {
None,
One(usize),
Many,
};
let mut used_index = UsedIndex::None;
// 0-15 -> index of a non-null branch // 0-15 -> index of a non-null branch
// 16 -> no non-null branch // 16 -> no non-null branch
// 17 -> multiple non-null branches // 17 -> multiple non-null branches
for i in 0..16 { for i in 0..16 {
match (nodes[i] == NULL_RLP, index[0]) { match (nodes[i] == NULL_RLP, &used_index) {
(false, _) => {}, (false, &UsedIndex::None) => used_index = UsedIndex::One(i),
(true, 16) => index[0] = i as u8, (false, &UsedIndex::One(_)) => used_index = UsedIndex::Many,
(true, _) => index[0] = 17, (_, _) => {},
} }
} }
match (index[0], node_value) { trace!("branch: used_index={:?}, node_value={:?}", used_index, node_value);
(16, None) => panic!("Branch with no subvalues. Something went wrong."), match (used_index, node_value) {
(0 ... 15, None) => { // one onward node (UsedIndex::None, None) => panic!("Branch with no subvalues. Something went wrong."),
(UsedIndex::One(a), None) => { // one onward node
// transmute to extension. // transmute to extension.
// TODO: OPTIMISE: - don't call fixed again but put the right node in straight away here. // TODO: OPTIMISE: - don't call fixed again but put the right node in straight away here.
// call fixed again since the transmute may cause invalidity. // call fixed again since the transmute may cause invalidity.
MaybeChanged::Changed(Self::encoded(self.fixed(Node::Extension(NibbleSlice::new_offset(&index[..], 1), nodes[index[0] as usize]), diff))) let new_partial: [u8; 1] = [a as u8; 1];
MaybeChanged::Changed(Self::encoded(self.fixed(Node::Extension(NibbleSlice::new_offset(&new_partial[..], 1), nodes[a as usize]), diff)))
}, },
(16, Some(value)) => { // one leaf value (UsedIndex::None, Some(value)) => { // one leaf value
// transmute to leaf. // transmute to leaf.
// call fixed again since the transmute may cause invalidity. // call fixed again since the transmute may cause invalidity.
MaybeChanged::Changed(Self::encoded(self.fixed(Node::Leaf(NibbleSlice::new(&b""[..]), value), diff))) MaybeChanged::Changed(Self::encoded(self.fixed(Node::Leaf(NibbleSlice::new(&b""[..]), value), diff)))
@ -657,42 +666,52 @@ impl TrieDB {
} }
fn cleared(&self, n: Node, partial: &NibbleSlice, diff: &mut Diff) -> Option<Bytes> { fn cleared(&self, n: Node, partial: &NibbleSlice, diff: &mut Diff) -> Option<Bytes> {
trace!("cleared (old: {:?}, partial: {:?})", n, partial); trace!("cleared old={:?}, partial={:?})", n, partial);
match (n, partial.is_empty()) { match (n, partial.is_empty()) {
(Node::Empty, _) => None, (Node::Empty, _) => None,
(Node::Branch(_, None), true) => { None }, (Node::Branch(_, None), true) => { None },
(Node::Branch(nodes, _), true) => Some(Self::encoded(self.fixed(Node::Branch(nodes, None), diff))), // matched as leaf-branch - give back fixed branch with it. (Node::Branch(payloads, _), true) => Some(Self::encoded(self.fixed(Node::Branch(payloads, None), diff))), // matched as leaf-branch - give back fixed branch with it.
(Node::Branch(nodes, value), false) => { (Node::Branch(payloads, value), false) => {
// Branch with partial left - route, clear, fix. // Branch with partial left - route, clear, fix.
let i: usize = partial.at(0) as usize; let i: usize = partial.at(0) as usize;
self.cleared(self.get_node(nodes[i]), &partial.mid(1), diff).map(|new_payload| { trace!("branch-with-partial node[{:?}]={:?}", i, payloads[i].pretty());
self.cleared(self.get_node(payloads[i]), &partial.mid(1), diff).map(|new_payload| {
trace!("branch-new-payload={:?}; delete-old={:?}", new_payload.pretty(), payloads[i].pretty());
// downsteam node needed to be changed. // downsteam node needed to be changed.
diff.delete_node_from_slice(nodes[i]); diff.delete_node_from_slice(payloads[i]);
// return fixed up new node. // return fixed up new node.
let mut new_nodes = nodes; let mut new_payloads = payloads;
new_nodes[i] = &new_payload; new_payloads[i] = &new_payload;
Self::encoded(self.fixed(Node::Branch(new_nodes, value), diff)) Self::encoded(self.fixed(Node::Branch(new_payloads, value), diff))
}) })
}, },
(Node::Leaf(node_partial, _), _) => { (Node::Leaf(node_partial, _), _) => {
trace!("leaf partial={:?}", node_partial);
match node_partial.common_prefix(partial) { match node_partial.common_prefix(partial) {
cp if cp == partial.len() => Some(Node::Empty.encoded()), // leaf to be deleted - delete it :) cp if cp == partial.len() => { // leaf to be deleted - delete it :)
trace!("matched-prefix (cp={:?}): REPLACE-EMPTY", cp);
Some(Node::Empty.encoded())
},
_ => None, // anything else and the key doesn't exit - no change. _ => None, // anything else and the key doesn't exit - no change.
} }
}, },
(Node::Extension(node_partial, node_payload), _) => { (Node::Extension(node_partial, node_payload), _) => {
trace!("extension partial={:?}, payload={:?}", node_partial, node_payload.pretty());
match node_partial.common_prefix(partial) { match node_partial.common_prefix(partial) {
cp if cp < partial.len() => None, // key in the middle of an extension - doesn't exist. cp if cp == node_partial.len() => {
_ => { trace!("matching-prefix (cp={:?}): SKIP,CLEAR,FIXUP", cp);
// key at end of extension - skip, clear, fix // key at end of extension - skip, clear, fix
self.cleared(self.get_node(node_payload), &partial.mid(node_partial.len()), diff).map(|new_payload| { self.cleared(self.get_node(node_payload), &partial.mid(node_partial.len()), diff).map(|new_payload| {
trace!("extension-new-payload={:?}; delete-old={:?}", new_payload.pretty(), node_payload.pretty());
// downsteam node needed to be changed. // downsteam node needed to be changed.
diff.delete_node_from_slice(node_payload); diff.delete_node_from_slice(node_payload);
// return fixed up new node. // return fixed up new node.
Self::encoded(self.fixed(Node::Extension(node_partial, &new_payload), diff)) Self::encoded(self.fixed(Node::Extension(node_partial, &new_payload), diff))
}) })
}, },
_ => None, // key in the middle of an extension - doesn't exist.
} }
}, },
} }
@ -735,13 +754,13 @@ mod tests {
env_logger::init().ok(); env_logger::init().ok();
let mut t1 = TrieDB::new_memory(); let mut t1 = TrieDB::new_memory();
t1.insert(&[0x01], &[0]);
t1.insert(&[0x01, 0x23], &[1]);
t1.insert(&[0x01, 0x34], &[2]); t1.insert(&[0x01, 0x34], &[2]);
t1.remove(&[0x01]);
let mut t2 = TrieDB::new_memory(); let mut t2 = TrieDB::new_memory();
t2.insert(&[0x01], &[0]);
t2.insert(&[0x01, 0x23], &[1]); t2.insert(&[0x01, 0x23], &[1]);
t2.insert(&[0x01, 0x34], &[2]); t2.insert(&[0x01, 0x34], &[2]);
t2.remove(&[0x01]);
t2.remove(&[0x01, 0x23]);
/*if t1.root() != t2.root()*/ { /*if t1.root() != t2.root()*/ {
trace!("{:?}", t1); trace!("{:?}", t1);
trace!("{:?}", t2); trace!("{:?}", t2);