Request for consensus testing

[zathras-crypto]

Hey @msgilligan (and @dexX7)

I was hoping to enlist your help and ask for an additional service to be added to the OmniJ consensus tests?

A test version of the new Omni API is available to query - is it simple to run consensus tests against it to verify accuracy?

Looking at what OmniChest provides I think the following is what you need:

https://azdgisclod.execute-api.us-west-2.amazonaws.com/test/getpropertybalances?propertyid=1

The URLs will be prettied before live & just replace propertyid as needed. The API is updated directly by an Omni Core node along with state changes so there should be no update lag time after processing blocks & transactions.

Thanks!
Z

[dexX7]

Hey @zathras-crypto: currently OmniJ uses the following endpoints of Chest:

• /mastercoin_verify/addresses.aspx
• /mastercoin_verify/properties.aspx
• /apireq.aspx?stat=customapireq_lastblockprocessed

Can you provide similar data with the new API? I'd give it a try, if so! :)

[zathras-crypto]

Thanks @dexX7 - no problem at all let me setup queries for those :)

Note - the getpropertybalances call above should basically be /mastercoin_verify/addresses.aspx, but with an additional propertyid field.

[dexX7]

On a related note: you may "namespace" the new API with v1, to have a chance for upgrades.

[zathras-crypto]

Ah, great idea! The API gateway lets you deploy in 'stages', so I can easily maintain backwards compatibility doing that! Awesome!

[dexX7]

Ah, that's so awesome! :) Really great stuff!

[zathras-crypto]

Ugh, it's been a while since I looked at 'Chests API - I see the properties endpoint still uses the currencyid attribute - would it be OK by OmniJ if I renamed this to propertyid?

[dexX7]

Yes, that's not an issue. Each "consensus provider" is handled seperately, so changing a few details is very doable: ChestConsensusTool.groovy

Speaking of, given the in-code comment, it appears as if it were useful, if the returned data from the API would have some form of timestamp (i.e. block height): when retrieving data, it's not clear to which point in time (in chain) the data refers to.

E.g.: my local block height is 12345, and I fetch some data via Chest. How could I know that the data also refers to block 12345? Currently the customapireq_lastblockprocessed is used to ensure the "block before fetch" and "block after fetch" are the same, basically as check to ensure that the actual data also refers to that block.

Maybe it would help to include the block in the results, too.

[zathras-crypto]

/mastercoin_verify/properties.aspx
to
https://vnccz8fm6e.execute-api.us-west-2.amazonaws.com/v1/getproperties

Speaking of, given the in-code comment, it appears as if it were useful, if the returned data from the API would have some form of timestamp (i.e. block height): when retrieving data, it's not clear to which point in time (in chain) the data refers to.

Sure - I can return the current block height in the response, but I can't guarantee that it's necessarily a snapshot of balances at that specific blockheight :(

Basically it's because the OmniCore node updates the DynamoDB tables as part of update_tally_map but the current block height isn't updated until the block end handler. So let's say a query to the API got processed after the Omni Core node handled a transaction that updated balances, but before it had finished processing the block - you'd then see the new balance but not yet the new block height.

I can put the block height update when the block is connected to the tip instead of after processing, but that just reverses the problem (you could see a new block height that doesn't necessarily contain all the balance updates for that block yet).

The timing is of course in ms but nevertheless I still can't definitively say "balances at block x are [...]". With that in mind, would it still be useful to include the block height? It's no trouble :)

[msgilligan]

An included block height would only be useful if it matches the balance data in the response. And, actually, the approach of:

1. bh1 = query block height
2. balances = query balances
3. bh2 = query block height
4. If bh1 != bh2, go to 1
5. return balances

also assumes that the block height being returned corresponds to the balances if it hasn't changed. It also assumes that all the balances in a single response are from the same block height.

For consensus testing what we really need is an atomic update. Is there a way to know for sure what block height the balances are for?

[zathras-crypto]

It also assumes that all the balances in a single response are from the same block height.

We don't have this guarantee from Omni Core, so it's not possible for OmniAPI to provide it sadly. All we can say is the balances are all from the same point in the state.

Let's look at what happens when we make a call for example for all balances for an ID:

1. The RPC request is received & parsed
2. The tally (map of all balances) is locked
3. The tally is looped and balance data added to a JSON object
4. The data is returned and the tally lock is released

So, this means that the data we are querying cannot be modified during the query (because of the lock), and thus will all be from the same point in the state. Unfortunately whilst very unlikely, it's possible that point might be mid-block, for example:

1. Block n is received & validated
2. Block hander starts looping the transactions
3. Some (not all) transactions have been processed
4. RPC server processes the balances request
5. Remainder of transactions in block are processed

This is feasible because RPC requests are processed in separate threads. I think it's very unlikely, but the higher the frequency of calls the more chance I guess.

For consensus testing what we really need is an atomic update. Is there a way to know for sure what block height the balances are for?

Sadly not, Omni Core can't provide a "state as of X block", only "the current state". Same with OmniAPI. We could lock the entire state at the start of processing a block and not release it until the block was finished processing (thus making all queries wait) but that seems awfully aggressive.

I think perhaps something like a USN might be suitable, but in the short term I think I can achieve this by adding to the blocks table with the handlers. I can add a status attribute for example. Then use the block begin handler to upload the block record as {"block":n,"blockhash":"s","status":"processing"} and then overwrite it with {"status":"processed"} in the block end handler.

If I configure up read consistency on the table that would provide a fairly safe model for OmniAPI to verify the latest block, check that it was finished processing, and then after retrieving data query again to make sure no new block has started processing (if it had run the function again). Then it could (I think) return a set of balances and be fairly confident in providing a blockheight for those balances.

Let me think some more on it :)

EDIT:

Sadly not, Omni Core can't provide a "state as of X block", only "the current state".

That's not strictly accurate - Omni Core can provide the state as of the last 50 blocks, we just currently don't have a way to expose anything other than "the current state". :)

[msgilligan]

Thanks for the detailed explanation, @zathras-crypto . (That should probably be documented somewhere -- if it isn't already.)

I think perhaps something like a USN might be suitable

I google "USN" and get "U.S. Navy". I don't know what you mean...

I can add a status attribute for example

Something like what you propose would certainly be helpful.

(So, I've been assuming incorrectly how things work and I don't think we've had any false alarms because of it.)

[zathras-crypto]

Thanks for the detailed explanation

Anytime :)

I google "USN" and get "U.S. Navy". I don't know what you mean...

Hehe - "Unique Sequence Number" - a value we can assign to each change to the state to track changes as atomic actions.

Something like what you propose would certainly be helpful.

Great, I'll start heading down that route :)

[dexX7]

it's possible that point might be mid-block, for example:

This makes me wonder. We immediately lock at the very beginning of each handler, i.e. when connecting a block, when disconnecting a block, or when processing a transaction, etc., but indeed, it basically looks like this:

1. begin block -- lock -- process -- unlock
2. process transaction (1) -- lock -- process -- unlock
3. process transaction (2) -- lock -- process -- unlock
4. process transaction (3) -- lock -- process -- unlock
5. ...
6. end block -- lock -- process -- unlock

During each step, it is - at least in theory - possible that a call goes through. In practise this may not be the case, because Bitcoin Core holds some locks during the block connecting, too, which may, or may not have an impact (even something silly where we log the current block height somewhere, which is dependent on the cs_main lock [which is held during the block connecting])..

We might move away from this approach and handle locks more controlled and manually, for example, we could lock when beginning a block, and unlock manually, when ending the block, but I fear this opens Pandora's box, and if anything, then I'd rather go in the opposite direction, and move away from "lock the whole handler", to "lock, only when we really access critical resources". This fueles of course the issue you initially outlined, that RPCs may go through, while the system is in a fuzzy state.

Let's step one step back: what would be the ideal scenario? Probably something where each operation is linked to a specific "piece of state". Prr.. and this kinda brings us back to OmniLayer/omnicore#254 (comment) and OmniLayer/omnicore#211 (comment), with "state snapshots".

[zathras-crypto]

State snapshots are the best approach for sure. Some form of UUID / sequence number and way to store actions atomically opens a lot of flexibility.

Pandora's box

Agreed, let's not manually unlock things :)

what would be the ideal scenario?

I'm not sure just how much of an issue this is since it relies on squeezing in an RPC call during changes to the state. RPC interaction has always been "the state as of now" rather than "the state as of block N" - long term if we store data well enough we could effectively query the state as of any block though I don't see this as a hugely useful outside of testing (as I think the vast majority of use-cases just want the latest data).