-
Notifications
You must be signed in to change notification settings - Fork 15
feat(ffi): add proposal root retrieval #910
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
Changes from all commits
8272677
d4f31af
b23483e
f9fc20c
5a793b1
cae4fc6
39f0e0c
2a1268c
a59bc25
5a4402a
60bddcd
5daf2fc
c1891ec
531b32a
074c5ec
e6afbef
c68a731
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -204,13 +204,19 @@ func kvForTest(i int) KeyValue { | |
// 1. By calling [Database.Propose] and then [Proposal.Commit]. | ||
// 2. By calling [Database.Update] directly - no proposal storage is needed. | ||
func TestInsert100(t *testing.T) { | ||
type dbView interface { | ||
Get(key []byte) ([]byte, error) | ||
Propose(keys, vals [][]byte) (*Proposal, error) | ||
Root() ([]byte, error) | ||
} | ||
|
||
tests := []struct { | ||
name string | ||
insert func(*Database, [][]byte, [][]byte) (root []byte, _ error) | ||
insert func(dbView, [][]byte, [][]byte) (dbView, error) | ||
}{ | ||
{ | ||
name: "Propose", | ||
insert: func(db *Database, keys, vals [][]byte) ([]byte, error) { | ||
name: "Propose and Commit", | ||
insert: func(db dbView, keys, vals [][]byte) (dbView, error) { | ||
proposal, err := db.Propose(keys, vals) | ||
if err != nil { | ||
return nil, err | ||
|
@@ -219,13 +225,28 @@ func TestInsert100(t *testing.T) { | |
if err != nil { | ||
return nil, err | ||
} | ||
return db.Root() | ||
return db, nil | ||
}, | ||
}, | ||
{ | ||
name: "Update", | ||
insert: func(db *Database, keys, vals [][]byte) ([]byte, error) { | ||
return db.Update(keys, vals) | ||
insert: func(db dbView, keys, vals [][]byte) (dbView, error) { | ||
actualDB, ok := db.(*Database) | ||
if !ok { | ||
return nil, fmt.Errorf("expected *Database, got %T", db) | ||
} | ||
_, err := actualDB.Update(keys, vals) | ||
return db, err | ||
}, | ||
}, | ||
{ | ||
name: "Propose", | ||
insert: func(db dbView, keys, vals [][]byte) (dbView, error) { | ||
proposal, err := db.Propose(keys, vals) | ||
if err != nil { | ||
return nil, err | ||
} | ||
return proposal, nil | ||
}, | ||
}, | ||
} | ||
|
@@ -240,20 +261,23 @@ func TestInsert100(t *testing.T) { | |
keys[i] = keyForTest(i) | ||
vals[i] = valForTest(i) | ||
} | ||
rootFromInsert, err := tt.insert(db, keys, vals) | ||
newDB, err := tt.insert(db, keys, vals) | ||
require.NoError(t, err, "inserting") | ||
|
||
for i := range keys { | ||
got, err := db.Get(keys[i]) | ||
got, err := newDB.Get(keys[i]) | ||
require.NoErrorf(t, err, "%T.Get(%q)", db, keys[i]) | ||
// Cast as strings to improve debug messages. | ||
want := string(vals[i]) | ||
require.Equal(t, want, string(got), "Recover nth batch-inserted value") | ||
} | ||
|
||
hash, err := db.Root() | ||
hash, err := newDB.Root() | ||
require.NoError(t, err, "%T.Root()", db) | ||
|
||
rootFromInsert, err := newDB.Root() | ||
require.NoError(t, err, "%T.Root() after insertion", db) | ||
|
||
// Assert the hash is exactly as expected. Test failure indicates a | ||
// non-hash compatible change has been made since the string was set. | ||
// If that's expected, update the string at the top of the file to | ||
|
@@ -415,16 +439,23 @@ func TestDeleteAll(t *testing.T) { | |
require.Empty(t, got, "Get(%d)", i) | ||
} | ||
|
||
emptyRootStr := expectedRoots[emptyKey] | ||
expectedHash, err := hex.DecodeString(emptyRootStr) | ||
require.NoError(t, err, "Decode expected empty root hash") | ||
|
||
// TODO: Check proposal root | ||
// Requires #918 | ||
// hash, err := proposal.Root() | ||
// require.NoError(t, err, "%T.Root() after commit", proposal) | ||
// require.Equalf(t, expectedHash, hash, "%T.Root() of empty trie", db) | ||
Comment on lines
+446
to
+450
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need to include this TODO here? If this requires 918, would it make more sense to include the added check in 918? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. #918 is an issue, but the code is trivial so we could remove that and fill it in when addressing the issue. |
||
|
||
// Commit the proposal. | ||
err = proposal.Commit() | ||
require.NoError(t, err, "Commit") | ||
|
||
// Check that the database is empty. | ||
hash, err := db.Root() | ||
require.NoError(t, err, "%T.Root()", db) | ||
expectedHashHex := expectedRoots[emptyKey] | ||
expectedHash, err := hex.DecodeString(expectedHashHex) | ||
require.NoError(t, err) | ||
require.Equalf(t, expectedHash, hash, "%T.Root() of empty trie", db) | ||
} | ||
|
||
|
@@ -445,27 +476,25 @@ func TestDropProposal(t *testing.T) { | |
err = proposal.Drop() | ||
require.NoError(t, err, "Drop") | ||
|
||
// Attempt to commit the dropped proposal. | ||
// Check all operations on the dropped proposal. | ||
err = proposal.Commit() | ||
require.ErrorIs(t, err, errDroppedProposal, "Commit(dropped proposal)") | ||
|
||
// Attempt to get a value from the dropped proposal. | ||
_, err = proposal.Get([]byte("non-existent")) | ||
require.ErrorIs(t, err, errDroppedProposal, "Get(dropped proposal)") | ||
_, err = proposal.Root() | ||
require.ErrorIs(t, err, errDroppedProposal, "Root(dropped proposal)") | ||
|
||
// Attempt to "emulate" the proposal to ensure it isn't internally available still. | ||
proposal = &Proposal{ | ||
handle: db.handle, | ||
id: 1, | ||
} | ||
|
||
// Check all operations on the fake proposal. | ||
_, err = proposal.Get([]byte("non-existent")) | ||
require.Contains(t, err.Error(), "proposal not found", "Get(fake proposal)") | ||
|
||
// Attempt to create a new proposal from the fake proposal. | ||
_, err = proposal.Propose([][]byte{[]byte("key")}, [][]byte{[]byte("value")}) | ||
require.Contains(t, err.Error(), "proposal not found", "Propose(fake proposal)") | ||
|
||
// Attempt to commit the fake proposal. | ||
err = proposal.Commit() | ||
require.Contains(t, err.Error(), "proposal not found", "Commit(fake proposal)") | ||
} | ||
|
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.
@aaronbuchwald I think this conflicts with something you're doing, so I can get rid of this or edit if you like
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.
No need to worry about a conflict here, will be easy to fix when we get to it