Skip to content
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

Refactor configs #154

Merged
merged 10 commits into from
Aug 1, 2019
Merged

Refactor configs #154

merged 10 commits into from
Aug 1, 2019

Conversation

m0ar
Copy link
Contributor

@m0ar m0ar commented Jul 19, 2019

This is still a bit unpolished, but I think it's in good enough order for someone to take to the finish line while I'm away 🏁 Sorry for leaving stuff unfinished, but the alternative was chillin' the last two days instead 😅

  • Does away with all TransformerConfig structs, the shiny new GetEventTransformerConfig polls all values automagically instead 💯
  • Does away with configuration "getter constants", except ABI's since I haven't been able to figure out how we can handle the signatures : (
  • Removes the checked.go set of constants, and renamed the labels.go entries to use snake_case we can utilise everywhere instead

One step closer to automagic generation of transformers from config 🙃

db/schema.sql Outdated
vat_fork integer DEFAULT 0 NOT NULL,
jug_init integer DEFAULT 0 NOT NULL,
yank integer DEFAULT 0 NOT NULL,
flip_tick integer DEFAULT 0 NOT NULL
);
Copy link
Contributor Author

@m0ar m0ar Jul 19, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dropping the _checked ending allows us to use the transformer labels as columns here.

Heads up that this requires a small change in vDB core, where it assumes all of these has _checked appended. This is probably why the integration test suite crashes, probably on the plugin test.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively we can append _checked to the strings sent from mcd_transformers into MarkHeaderChecked et al, but I don't love the _checked bit anyway. Your choice!

[contract.COL1_FLIP]
address = "0x911c1bb935ce7fe5acb8069acd52d50d3c896353"
abi = '[{"constant":false,"inputs":[{"name":"id","type":"uint256"}],"name":"yank","outputs":[],"payable":false,"stateMutability":"nonpayable","type":"function"},{"constant":false,"inputs":[{"name":"what","type":"bytes32"},{"name":"data","type":"uint256"}],"name":"file","outputs":[],"payable":false,"stateMutability":"nonpayable","type":"function"},{"constant":false,"inputs":[{"name":"usr","type":"address"},{"name":"gal","type":"address"},{"name":"tab","type":"uint256"},{"name":"lot","type":"uint256"},{"name":"bid","type":"uint256"}],"name":"kick","outputs":[{"name":"id","type":"uint256"}],"payable":false,"stateMutability":"nonpayable","type":"function"},{"constant":true,"inputs":[],"name":"vat","outputs":[{"name":"","type":"address"}],"payable":false,"stateMutability":"view","type":"function"},{"constant":true,"inputs":[{"name":"","type":"uint256"}],"name":"bids","outputs":[{"name":"bid","type":"uint256"},{"name":"lot","type":"uint256"},{"name":"guy","type":"address"},{"name":"tic","type":"uint48"},{"name":"end","type":"uint48"},{"name":"usr","type":"address"},{"name":"gal","type":"address"},{"name":"tab","type":"uint256"}],"payable":false,"stateMutability":"view","type":"function"},{"constant":false,"inputs":[{"name":"id","type":"uint256"},{"name":"lot","type":"uint256"},{"name":"bid","type":"uint256"}],"name":"tend","outputs":[],"payable":false,"stateMutability":"nonpayable","type":"function"},{"constant":true,"inputs":[],"name":"ttl","outputs":[{"name":"","type":"uint48"}],"payable":false,"stateMutability":"view","type":"function"},{"constant":false,"inputs":[{"name":"id","type":"uint256"},{"name":"lot","type":"uint256"},{"name":"bid","type":"uint256"}],"name":"dent","outputs":[],"payable":false,"stateMutability":"nonpayable","type":"function"},{"constant":false,"inputs":[{"name":"usr","type":"address"}],"name":"rely","outputs":[],"payable":false,"stateMutability":"nonpayable","type":"function"},{"constant":true,"inputs":[],"name":"beg","outputs":[{"name":"","type":"uint256"}],"payable":false,"stateMutability":"view","type":"function"},{"constant":false,"inputs":[{"name":"usr","type":"address"}],"name":"deny","outputs":[],"payable":false,"stateMutability":"nonpayable","type":"function"},{"constant":true,"inputs":[{"name":"","type":"address"}],"name":"wards","outputs":[{"name":"","type":"uint256"}],"payable":false,"stateMutability":"view","type":"function"},{"constant":true,"inputs":[],"name":"ilk","outputs":[{"name":"","type":"bytes32"}],"payable":false,"stateMutability":"view","type":"function"},{"constant":false,"inputs":[{"name":"id","type":"uint256"}],"name":"deal","outputs":[],"payable":false,"stateMutability":"nonpayable","type":"function"},{"constant":true,"inputs":[],"name":"tau","outputs":[{"name":"","type":"uint48"}],"payable":false,"stateMutability":"view","type":"function"},{"constant":true,"inputs":[],"name":"kicks","outputs":[{"name":"","type":"uint256"}],"payable":false,"stateMutability":"view","type":"function"},{"constant":false,"inputs":[{"name":"id","type":"uint256"}],"name":"tick","outputs":[],"payable":false,"stateMutability":"nonpayable","type":"function"},{"inputs":[{"name":"vat_","type":"address"},{"name":"ilk_","type":"bytes32"}],"payable":false,"stateMutability":"nonpayable","type":"constructor"},{"anonymous":false,"inputs":[{"indexed":false,"name":"id","type":"uint256"},{"indexed":false,"name":"lot","type":"uint256"},{"indexed":false,"name":"bid","type":"uint256"},{"indexed":false,"name":"tab","type":"uint256"},{"indexed":true,"name":"usr","type":"address"},{"indexed":true,"name":"gal","type":"address"}],"name":"Kick","type":"event"},{"anonymous":true,"inputs":[{"indexed":true,"name":"sig","type":"bytes4"},{"indexed":true,"name":"usr","type":"address"},{"indexed":true,"name":"arg1","type":"bytes32"},{"indexed":true,"name":"arg2","type":"bytes32"},{"indexed":false,"name":"data","type":"bytes"}],"name":"LogNote","type":"event"}]'
deployed = 11579891 # deployment block of ETH_FLIP_A contract-- on update, ensure that it's still the first one deployed
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file is just composeAndExecute with a bit different structure (and a better name 😁 ).

The structural changes makes lookup of contract metadata a bit more self-explanatory, is in line with the formatting in the exporter section, and it'll be easier to see which contracts changes refer to.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, and all these FLIPs have the same deployment block, but GetMinDeploymentBlock (this dude) will take the lowest one.

It's a necessity that all entries have the abi field, even though it's identical for the FLIP family. This way we can blindly lookup ABIs for the contracts referenced in the exporter section:

    [exporter.tend]
        path = "transformers/events/tend/initializer"
        type = "eth_event"
        repository = "github.com/vulcanize/mcd_transformers"
        migrations = "db/migrations"
        contracts = [ "ETH_FLIP_A", "ETH_FLIP_B", "ETH_FLIP_C",
                      "COL1_FLIP", "COL2_FLIP", "COL3_FLIP", "COL4_FLIP", "COL5_FLIP",
                    ]
        rank = "0"

The code eating this contract cross-reference field and looking up the ABIs is here, and actually checks that they are identical when used in the same transformer 💪

StartingBlockNumber: constants.CatDeploymentBlock(),
EndingBlockNumber: -1,
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of these dudes disappear since we can generate them freely given just the transformer label (vow_fess) and the corresponding signature 💯

func GetEventTransformerConfig(transformerLabel, signature string) transformer.EventTransformerConfig {
	contractNames := constants.GetTransformerContractNames(transformerLabel)
	return transformer.EventTransformerConfig{
		TransformerName:     transformerLabel,
		ContractAddresses:   constants.GetContractAddresses(contractNames),
		ContractAbi:         constants.GetContractsABI(contractNames),
		Topic:               signature,
		StartingBlockNumber: constants.GetMinDeploymentBlock(contractNames),
		EndingBlockNumber:   -1,
	}
}

func Col4FlipContractAddress() string { return getEnvironmentString("contract.address.COL4_FLIP") }
func Col5FlipContractAddress() string { return getEnvironmentString("contract.address.COL5_FLIP") }
func FlipperContractAddresses() []string {
return []string{EthFlipContractAddressA(), EthFlipContractAddressB(), EthFlipContractAddressC(), Col1FlipContractAddress(), Col2FlipContractAddress(), Col3FlipContractAddress(), Col4FlipContractAddress(), Col5FlipContractAddress()}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔥

log.WithField("contracts", contractNames).Fatalf("ABIs not consistent between contracts")
}
}
return abi
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When a transformer has several contracts listed, it checks that they all have the same ABI

@yaoandrew yaoandrew force-pushed the refactor-configs branch 2 times, most recently from 248183f to fcdc246 Compare July 31, 2019 20:20
Copy link
Contributor

@rmulhol rmulhol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! The only things that I think are blockers are (1) passing the correct SpotPokeLabel to MarkHeaderChecked on the SpotPokeRepository (rather than BiteLabel 🙈) and (2) passing the contract address to the mappings + repository in the flap storage initializer

transformers/events/spot_poke/repository_test.go Outdated Show resolved Hide resolved
transformers/events/spot_poke/repository.go Outdated Show resolved Hide resolved
transformers/integration_tests/bite.go Show resolved Hide resolved
transformers/integration_tests/cat_file.go Show resolved Hide resolved
transformers/integration_tests/deal.go Outdated Show resolved Hide resolved
transformers/storage/flap/initializer/initializer.go Outdated Show resolved Hide resolved
transformers/storage/flap/storage_keys_lookup_test.go Outdated Show resolved Hide resolved
transformers/test_data/tend.go Outdated Show resolved Hide resolved
transformers/test_data/config_values.go Outdated Show resolved Hide resolved
transformers/test_data/vat_file.go Outdated Show resolved Hide resolved
Copy link

@elizabethengelman elizabethengelman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good! Just a few small comments/questions.

transformers/events/bite/initializer/initializer.go Outdated Show resolved Hide resolved
transformers/integration_tests/bite.go Show resolved Hide resolved
transformers/integration_tests/cat_file.go Show resolved Hide resolved
transformers/shared/config.go Outdated Show resolved Hide resolved
transformers/storage/flap/initializer/initializer.go Outdated Show resolved Hide resolved
transformers/test_data/flap_kick.go Outdated Show resolved Hide resolved
@yaoandrew yaoandrew merged commit c7fe3b1 into staging Aug 1, 2019
@yaoandrew yaoandrew deleted the refactor-configs branch August 1, 2019 19:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants