change undo stack processing order to created-modified-removed

[raymondnuaa]

With current undo stacks processing order: modified, created, removed, there will be a dead loop for the following example code.

struct book : public chainbase::object<0, book> {
   template<typename Constructor, typename Allocator>
    book(  Constructor&& c, Allocator&& a ) {
       c(*this);
    }

    id_type id;
    id_type  a;
};

typedef multi_index_container<
  book,
  indexed_by<
     ordered_unique< member<book,book::id_type,&book::id> >,
     ordered_unique< member<book,book::id_type,&book::a> >,
     ordered_non_unique< BOOST_MULTI_INDEX_MEMBER(book,int,b) >
  >,
  chainbase::allocator<book>
> book_index;

const auto& new_book = db.create<book>( []( book& b ) {
    b.a = 10;
 } );

auto session = db.start_undo_session(true);

db.modify( new_book, [&]( book& b ) {
   b.a = 20;
});

const auto& another_book = db.create<book>( []( book& b ) {
    b.a = 10;
} );

db.undo();

So we need to change the undo stacks processing order to created, modified, removed. The issue in above code can be fixed.

Actually there are four cases to make the unique key conflict. Say we have two records (A, B) in one table which has unique key. The four cases are:

1. remove A, then create new record C which will use A's original value for C's the unique key
2. remove A, then modify B's unique key value to A's
3. modify A (change its unique key to other value), then create new record C which use A's original value as its unique key
4. modify A (change its unique key to other value), then modify B's unique key value to A's

With original undo stack processing order (modified, created, removed), case 3) will has issue which is showed in the example code above.

With the changed undo stack processing order (created, modified, removed), case 1) 2) 3) all work fine.

The case 4) is the most complicated one. You can even make the modified undo stack has two records that A and B have swapped their original value. This fix can not handle this case.

[tbfleming]

Have you found a way to cause nodeos to hit a currently-mishandled case?

Have you verified that nodeos’s behavior won’t change with your proposed ordering?

[raymondnuaa]

I went through some indices and did not find a way to trigger such case in nodeos.

I managed to run the chainbase test case in this repo (need some change in CMakelists and allow dirty for db2 ), all passed.

Also run a 5 nodes local test net of nodeos, it works fine as before with the changed undo order.

[heifner]

Thanks @raymondnuaa. Subsumed by #44 .