From 224251f916b01e5aef77cc119160253a6678cec7 Mon Sep 17 00:00:00 2001 From: Alex Lunyov Date: Tue, 12 Nov 2019 17:09:30 +0800 Subject: [PATCH] feat: add feature flags (#351) * Impelement feature flags module * Make flags serializable * Print spend_cond * Simplify flag value func * Flag for stricter spending conditions rules that will allow to sync with testnet * Updated readme * Update src/flags/index.js Co-Authored-By: Kosta Korenkov * Typo --- README.md | 32 +++++++++++ src/bridgeState.js | 5 +- src/flags/flags.test.js | 76 +++++++++++++++++++++++++++ src/flags/flagsFactory.js | 31 +++++++++++ src/flags/index.js | 9 ++++ src/tx/applyTx/checkSpendCond.js | 40 ++++++++------ src/tx/applyTx/checkSpendCond.test.js | 4 ++ src/tx/index.js | 2 +- src/txHelpers/printTx.js | 3 ++ 9 files changed, 185 insertions(+), 17 deletions(-) create mode 100644 src/flags/flags.test.js create mode 100644 src/flags/flagsFactory.js create mode 100644 src/flags/index.js diff --git a/README.md b/README.md index bc991e25..827d208f 100644 --- a/README.md +++ b/README.md @@ -79,6 +79,38 @@ Your local port `9999` forwards now to the remote host on address `127.0.0.1` an - `network` — plasma network name - `networkId` - network ID. Possible values: `1340` - Leap mainnet, `1341` - Leap testnet. - `peers` — array of peers +- `flagHeights` — `flag → height` mapping to enable feature flags only on certain heights (see [feature flags section](#feature-flags)) + +### Feature flags + +If you want to introduce breaking changes (it can break existing networks) in tx checks you should wrap these changes into condition with feature flag to avoid [resync problems](https://github.com/leapdao/leap-node/issues/334). + +```es6 +if (bridgeState.flags.tx_should_fail_on_zero_input) { + if (valueOf(input) === 0) { + throw new Error('WTF'); + } +} +``` + +#### How to add a new flag + +1. Add it into `FLAGS` array [here](src/flags/index.js#L3) +2. That’s all, you can use it. It will be `true` by default + +#### How to configure feature flags + +1. Add `flagHeights` section into the network config +2. Add activation height: +``` +{ + ..., + "flagHeights": { + "tx_should_fail_on_zero_input": 5000, + } +} +``` +3. Until this height flag will be `false`, after (inclusively) — `true` ### Config presets diff --git a/src/bridgeState.js b/src/bridgeState.js index 28ec237c..3c2d3d09 100644 --- a/src/bridgeState.js +++ b/src/bridgeState.js @@ -19,6 +19,7 @@ const singleOperatorABI = require('./abis/singleOperator'); let operatorABI = require('./abis/operator'); const proxyABI = require('./abis/proxy'); const { NFT_COLOR_BASE, NST_COLOR_BASE } = require('./api/methods/constants'); +const flags = require('./flags'); module.exports = class BridgeState { constructor(db, privKey, config, relayBuffer) { @@ -65,6 +66,8 @@ module.exports = class BridgeState { this.epochLengths = []; this.minGasPrices = []; + this.flags = flags(this, config.flagHeights); + this.onNewBlock = this.onNewBlock.bind(this); this.eventsBuffer = new TinyQueue([], (a, b) => { if (a.blockNumber === b.blockNumber) { @@ -204,7 +207,7 @@ module.exports = class BridgeState { ); this.exitEventSubscription.on('newEvents', this.handleExitingUtxos); this.exitEventSubscription.init(); - + logNode('Synced'); } diff --git a/src/flags/flags.test.js b/src/flags/flags.test.js new file mode 100644 index 00000000..cc6eab96 --- /dev/null +++ b/src/flags/flags.test.js @@ -0,0 +1,76 @@ +const flagsFactory = require('./flagsFactory'); + +const makeFlags = flagsFactory(['test_flag', 'test_flag_2']); + +describe('Feature flags', () => { + it('should be true by default', () => { + const bridgeState = { blockHeight: 0 }; + const flags = makeFlags(bridgeState, {}); + + expect(flags.test_flag).toBe(true); + }); + + it('should be false if configured height not reached yet', () => { + const bridgeState = { blockHeight: 5 }; + const flags = makeFlags(bridgeState, { + test_flag: 10, + }); + + expect(flags.test_flag).toBe(false); + }); + + it('should be true if configured height reached', () => { + const bridgeState = { blockHeight: 10 }; + const flags = makeFlags(bridgeState, { + test_flag: 10, + }); + + expect(flags.test_flag).toBe(true); + }); + + it('should change value after height change', () => { + const bridgeState = { blockHeight: 0 }; + const flags = makeFlags(bridgeState, { + test_flag: 10, + }); + + expect(flags.test_flag).toBe(false); + bridgeState.blockHeight = 10; + expect(flags.test_flag).toBe(true); + }); + + it('should throw on unknown flag', () => { + const bridgeState = { blockHeight: 0 }; + const flags = makeFlags(bridgeState); + expect(() => { + const value = flags.unknown_flag; // eslint-disable-line no-unused-vars + }).toThrow('Unknown feature flag unknown_flag'); + }); + + it('should throw on attempt to set a flag value', () => { + const bridgeState = { blockHeight: 0 }; + const flags = makeFlags(bridgeState); + expect(() => { + flags.test_flag = true; + }).toThrow('Flags are read-only: test_flag'); + }); + + it('should serialize flag values', () => { + const bridgeState = { blockHeight: 0 }; + const flags = makeFlags(bridgeState, { + test_flag: 10, + }); + + bridgeState.blockHeight = 0; + expect(JSON.parse(JSON.stringify(flags))).toEqual({ + test_flag: false, + test_flag_2: true, + }); + + bridgeState.blockHeight = 10; + expect(JSON.parse(JSON.stringify(flags))).toEqual({ + test_flag: true, + test_flag_2: true, + }); + }); +}); diff --git a/src/flags/flagsFactory.js b/src/flags/flagsFactory.js new file mode 100644 index 00000000..c2d53c52 --- /dev/null +++ b/src/flags/flagsFactory.js @@ -0,0 +1,31 @@ +/* eslint-disable no-prototype-builtins */ + +function flagValue(bridgeState, flagHeights, flag) { + const targetHeight = flagHeights[flag] || 0; + return bridgeState.blockHeight >= targetHeight; +} + +module.exports = (flags = []) => (bridgeState, flagHeights = {}) => { + const proxy = new Proxy(flagHeights, { + get(target, key) { + if (key === 'toJSON') { + return () => + flags.reduce((flagValues, flag) => { + flagValues[flag] = flagValue(bridgeState, target, flag); + return flagValues; + }, {}); + } + + if (!flags.includes(key)) { + throw new Error(`Unknown feature flag ${key}`); + } + + return flagValue(bridgeState, target, key); + }, + set(_, key) { + throw new Error(`Flags are read-only: ${key}`); + }, + }); + + return proxy; +}; diff --git a/src/flags/index.js b/src/flags/index.js new file mode 100644 index 00000000..54008c62 --- /dev/null +++ b/src/flags/index.js @@ -0,0 +1,9 @@ +const flagsFactory = require('./flagsFactory'); + +// when adding a flag, add a link to the issue/PR describing the reason for the change +const FLAGS = [ + 'spend_cond_stricter_rules', // https://github.com/leapdao/leap-node/pull/303 + 'spend_cond_new_bytecode' // https://github.com/leapdao/leap-node/pull/292 +]; + +module.exports = flagsFactory(FLAGS); diff --git a/src/tx/applyTx/checkSpendCond.js b/src/tx/applyTx/checkSpendCond.js index 7a92e664..8fe8d3a0 100644 --- a/src/tx/applyTx/checkSpendCond.js +++ b/src/tx/applyTx/checkSpendCond.js @@ -262,12 +262,20 @@ module.exports = async (state, tx, bridgeState, nodeConfig = {}) => { let allowance; if (!spendingIsOwner) { - if (input.signer) { - if (unspent.address !== input.signer) { - throw new Error( - `output owner ${unspent.address} unequal input signer: ${input.signer}` - ); + if (input.signer && unspent.address !== input.signer) { + throw new Error( + `output owner ${unspent.address} unequal input signer: ${input.signer}` + ); + } + + // will throw if allowance is undefined + // stricter rule was introduced here + // https://github.com/leapdao/leap-node/blob/b534718807214318acaa6a9ff88b9cb8f1780ef1/src/tx/applyTx/checkSpendCond.js#L271 + if (bridgeState.flags.spend_cond_stricter_rules) { + if (input.signer) { + allowance = {}; } + } else { allowance = {}; } } else { @@ -281,10 +289,9 @@ module.exports = async (state, tx, bridgeState, nodeConfig = {}) => { tokenValueBuf, utils.toBuffer(unspent.data), ]); - bytecode = - bridgeState.networkId === 218508104 && bridgeState.blockHeight < 41000 - ? ERC1948_BYTECODE_218508104 - : ERC1948_BYTECODE; + bytecode = bridgeState.flags.spend_cond_new_bytecode + ? ERC1948_BYTECODE + : ERC1948_BYTECODE_218508104; if (allowance) { allowance = { @@ -519,12 +526,15 @@ module.exports = async (state, tx, bridgeState, nodeConfig = {}) => { logOuts.push(new Output(amount, owner, deployed[originAddr])); } }); - iterateBag(nftBag, (originAddr, owner) => { - if (!nftBag[originAddr][owner].touched) { - // throw instead of return, because of the cb function - throw new Error(`not touched ${nftBag[originAddr][owner].addr}`); - } - }); + + if (bridgeState.flags.spend_cond_stricter_rules) { + iterateBag(nftBag, (originAddr, owner) => { + if (!nftBag[originAddr][owner].touched) { + // throw instead of return, because of the cb function + throw new Error(`not touched ${nftBag[originAddr][owner].addr}`); + } + }); + } const gasUsed = BigInt(evmResult.gasUsed); // XXX: Fixed gasPrice for now. We include it again in the tx format as the next breaking change. diff --git a/src/tx/applyTx/checkSpendCond.test.js b/src/tx/applyTx/checkSpendCond.test.js index e5e71ca5..837742bd 100644 --- a/src/tx/applyTx/checkSpendCond.test.js +++ b/src/tx/applyTx/checkSpendCond.test.js @@ -2,6 +2,7 @@ const { Tx, Input, Outpoint, Output } = require('leap-core'); const utils = require('ethereumjs-util'); const ethers = require('ethers'); // eslint-disable-line import/no-extraneous-dependencies const checkSpendCond = require('./checkSpendCond'); +const makeFlags = require('../../flags'); const { NFT_COLOR_BASE, NST_COLOR_BASE, @@ -34,6 +35,9 @@ const bridgeState = { logsCache: {}, web3: new Web3(), }; +bridgeState.flags = makeFlags(bridgeState, { + spend_cond_new_bytecode: 1000, +}); const NFTCondition = '6080604052348015600f57600080fd5b5060043610602b5760e060020a6000350463d01a81e181146030575b600080fd5b605960048036036040811015604457600080fd5b50600160a060020a038135169060200135605b565b005b6040805160e060020a6323b872dd028152306004820152600160a060020a03841660248201526044810183905290517311111111111111111111111111111111111111119182916323b872dd9160648082019260009290919082900301818387803b15801560c857600080fd5b505af115801560db573d6000803e3d6000fd5b5050505050505056fea165627a7a723058206e658cc8b44fd3d32eef8c4222a0f8773e93bc6efa0fb08e4db77979dacc9e790029'; diff --git a/src/tx/index.js b/src/tx/index.js index d0218212..41e7d909 100644 --- a/src/tx/index.js +++ b/src/tx/index.js @@ -25,7 +25,7 @@ module.exports = (bridgeState, nodeConfig) => async ( await applyTx(state, tx, bridgeState, nodeConfig); accumulateTx(state, tx); } catch (err) { - logError(err.message); + logError(err.message || err); throw err; } diff --git a/src/txHelpers/printTx.js b/src/txHelpers/printTx.js index 6a027167..013dc29f 100644 --- a/src/txHelpers/printTx.js +++ b/src/txHelpers/printTx.js @@ -49,6 +49,9 @@ module.exports = (state, tx) => { return `periodVote: slotId: ${tx.options.slotId}, root: ${bufferToHex(tx.inputs[0].prevout.hash)}`; } + case Type.SPEND_COND: { + return 'SPEND_COND'; + } default: return undefined; }