-
Notifications
You must be signed in to change notification settings - Fork 219
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
feat(core)!: define OutputFlags for side-chain contracts #4088
feat(core)!: define OutputFlags for side-chain contracts #4088
Conversation
77fc23b
to
64033c4
Compare
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.
LGTM - barring the CI failures
Same here, LGTM, I believe they're exactly the features we need for the contract lifecycle |
/// Output signals validator node acceptance to run a contract. | ||
const CONTRACT_ACCEPT = 0x0400; | ||
/// Output is a contract checkpoint. | ||
const CONTRACT_CHECKPOINT = 0x0800; |
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.
We may need an additional flag for the initial checkpoint where all the contract acceptance UTXOs have to be spent into it - a slight nuance on the rules to be enforced.
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.
Isn't that going to be just the first checkpoint? Or does it need to be a custom output for that?
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.
I think you're right and we might need an abandoned flag so the asset owner can spend an abandoned checkpoint into that state to activate the emergency keys
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.
Agreed
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.
I left off the INITIAL_CHECKPOINT flag for now because even if we have it, we'd have to check that it is an INITIAL checkpoint (we can't trust the flag), in doing so we have done the work to detect it.
64033c4
to
87ce0ac
Compare
1a882aa
to
3464e21
Compare
f7e11fa
to
c05327a
Compare
c05327a
to
06f97e6
Compare
…or consensus enc changes (#4098) Description --- - mine a new nonce for `check_difficulty` and `check_share` tests - adds `detect_change_in_consensus_encoding` to detect changes in consensus encoding that require updates to pool miner code Motivation and Context --- Ref PR #4088 - `check_difficulty` and `check_share` tests are brittle because any change to consensus encoding causes them to fail. In that case, OutputFlags changed from a u8 to a u16. These tests now generate a new nonce and difficulty (takes ~1s on good hardware) and check that the FFI functions return matching results. This was done to eliminate the time and LOC needed to fix these tests in PRs that require changes to consensus encoding (many expected in the coming weeks/months). `detect_change_in_consensus_encoding` was added to make it clear that code changes in a given PR require an update to the pool miner code. How Has This Been Tested? --- The new tests pass
Description
Defines OutputFlags for side-chain contracts.
Motivation and Context
Define flags required for side-chain contracts. Deprecated flags are kept and can be removed in implementation PRs
for the various side-chain components.
How Has This Been Tested?
Unit and cucumber tests updated as necessary